Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include Go version in name of artifact (tar.gz, zip) #140

Closed
dagood opened this issue Jul 16, 2021 · 7 comments · Fixed by #990
Closed

Include Go version in name of artifact (tar.gz, zip) #140

dagood opened this issue Jul 16, 2021 · 7 comments · Fixed by #990

Comments

@dagood
Copy link
Member

dagood commented Jul 16, 2021

We should consider including the Go version in these names (when we are building a versioned branch):

  • go.20210716.4.linux-amd64.tar.gz
  • go.20210716.4.windows-amd64.zip

The official tar.gz/zip files look like this:

  • go1.16.6.linux-amd64.tar.gz
  • go1.16.6.windows-amd64.zip

Upstream checks in a VERSION file when they fork a branch, then update it with every patch release:

go/VERSION

Line 1 in bc51e93

go1.16.6

We could use that file to include both the Go version and Microsoft build number: go1.16.6.20210716.4.linux-amd64.tar.gz.

Or we could remove the build number to match upstream perfectly, however we should be sure that you can trace a given Go build back to the AzDO build that produced it. (We could simply put our own new file with this info into GOROOT, as a sibling of the existing VERSION file in GOROOT.)

I don't know of any particularly strong reasons to do this, though:

  • Potentially easier drop-in replacement if we match the upstream artifact name exactly.
  • Easier to tell which tar.gz is which if you've downloaded a few of them over time. (Build number is not mappable back to a branch name without looking for the build on AzDO and seeing which branch it used.)
@dagood
Copy link
Member Author

dagood commented Aug 9, 2021

(We could simply put our own new file with this info into GOROOT, as a sibling of the existing VERSION file in GOROOT.)

We should do something like this either way, to make it easy to see which Microsoft build you have even if the tar.gz is already extracted. (E.g. in a Docker image.)

@jaredpar
Copy link
Member

Or we could remove the build number to match upstream perfectly, however we should be sure that you can trace a given Go build back to the AzDO build that produced it. (We could simply put our own new file with this info into GOROOT, as a sibling of the existing VERSION file in GOROOT.)

This makes the most sense to me. It aligns with our goal of making the process of moving from the official binaries / images to the ones that we produce as simple as changing the URL they're grabbing from.

We should do something like this either way, to make it easy to see which Microsoft build you have even if the tar.gz is already extracted. (E.g. in a Docker image.)

I think we should consider adding the Microsoft name + build number into commands like go version. Essentially a command that is very easy to print out during CI and show up in logs (to allow customers to verify at a glance that they're using the correct binaries).

@dagood
Copy link
Member Author

dagood commented Sep 3, 2021

I think we should consider adding the Microsoft name + build number into commands like go version.

👍 I poked around, and found a few places to start figuring out the next steps:

go/src/runtime/extern.go

Lines 246 to 261 in 1e06f77

// buildVersion is the Go tree's version string at build time.
//
// If any GOEXPERIMENTs are set to non-default values, it will include
// "X:<GOEXPERIMENT>".
//
// This is set by the linker.
//
// This is accessed by "go version <binary>".
var buildVersion string
// Version returns the Go tree's version string.
// It is either the commit hash and date at the time of the build or,
// when possible, a release tag like "go1.3".
func Version() string {
return buildVersion
}

// The \xff is invalid UTF-8, meant to make it less likely
// to find one of these accidentally.
const prefix = "\xff Go buildinf:" // 14 bytes, plus 2 data bytes filled in below
data := make([]byte, 32)
copy(data, prefix)
data[len(prefix)] = byte(ctxt.Arch.PtrSize)
data[len(prefix)+1] = 0
if ctxt.Arch.ByteOrder == binary.BigEndian {
data[len(prefix)+1] = 1
}
s.SetData(data)
s.SetSize(int64(len(data)))
r, _ := s.AddRel(objabi.R_ADDR)
r.SetOff(16)
r.SetSiz(uint8(ctxt.Arch.PtrSize))
r.SetSym(ldr.LookupOrCreateSym("runtime.buildVersion", 0))
r, _ = s.AddRel(objabi.R_ADDR)
r.SetOff(16 + int32(ctxt.Arch.PtrSize))
r.SetSiz(uint8(ctxt.Arch.PtrSize))
r.SetSym(ldr.LookupOrCreateSym("runtime.modinfo", 0))

// The build info blob left by the linker is identified by
// a 16-byte header, consisting of buildInfoMagic (14 bytes),
// the binary's pointer size (1 byte),
// and whether the binary is big endian (1 byte).
var buildInfoMagic = []byte("\xff Go buildinf:")
// findVers finds and returns the Go version and module version information
// in the executable x.
func findVers(x exe) (vers, mod string) {
// Read the first 64kB of text to find the build info blob.
text := x.DataStart()
data, err := x.ReadData(text, 64*1024)
if err != nil {
return
}
for ; !bytes.HasPrefix(data, buildInfoMagic); data = data[32:] {
if len(data) < 32 {
return
}
}
// Decode the blob.
ptrSize := int(data[14])
bigEndian := data[15] != 0
var bo binary.ByteOrder
if bigEndian {
bo = binary.BigEndian
} else {
bo = binary.LittleEndian
}
var readPtr func([]byte) uint64
if ptrSize == 4 {
readPtr = func(b []byte) uint64 { return uint64(bo.Uint32(b)) }
} else {
readPtr = bo.Uint64
}
vers = readString(x, ptrSize, readPtr, readPtr(data[16:]))
if vers == "" {
return
}
mod = readString(x, ptrSize, readPtr, readPtr(data[16+ptrSize:]))
if len(mod) >= 33 && mod[len(mod)-17] == '\n' {
// Strip module framing.
mod = mod[16 : len(mod)-16]
} else {
mod = ""
}
return

@dagood
Copy link
Member Author

dagood commented Oct 1, 2021

I tried something super simple here:

buildVersion := buildcfg.Version
if goexperiment := buildcfg.GOEXPERIMENT(); goexperiment != "" {
buildVersion += " X:" + goexperiment
}
addstrdata1(ctxt, "runtime.buildVersion="+buildVersion)

 	}
+	buildVersion += " Hello from dagood"
 	addstrdata1(ctxt, "runtime.buildVersion="+buildVersion)

But, that gives me a bunch of "stale dependency" errors: https://gist.github.com/dagood/c6a870e9eb8859469076aa0babc90781

...
HASH[build runtime/internal/sys]
HASH[build runtime/internal/sys]: "go1.17.1 Hello from dagood"
HASH[build runtime/internal/sys]: "compile\n"
HASH[build runtime/internal/sys]: "goos linux goarch amd64\n"
HASH[build runtime/internal/sys]: "import \"runtime/internal/sys\"\n"
HASH[build runtime/internal/sys]: "omitdebug false standard true local false prefix \"\"\n"
HASH[build runtime/internal/sys]: "modinfo \"\"\n"
HASH[build runtime/internal/sys]: "compile compile version go1.17.1 [] []\n"
HASH[build runtime/internal/sys]: "=\n"
HASH /home/dagood/git/go/src/runtime/internal/sys/arch.go: ce22176d2caf2b8aa9bbf63da6fc175efb8439ae1ba0e2d45eac8c05cb3a3054
HASH[build runtime/internal/sys]: "file arch.go ziIXbSyvK4qpu_Y9pvwX\n"
...
go tool dist: unexpected stale targets reported by /home/dagood/git/go/bin/go list -gcflags="" -ldflags="" for [std cmd] (consider rerunning with GOMAXPROCS=1 GODEBUG=gocachehash=1):
	STALE archive/tar: stale dependency: internal/cpu
	STALE archive/zip: stale dependency: internal/cpu
...

Adding our extra string this way is modifying the linker (and anything built by "our" linker), and the build version seems to be included in content hash checks. I'm not quite sure where exactly the checks are taking place. The build has already gone through a few rounds of bootstrapping at this point, so I wouldn't have thought it would be an issue.

I found golang/go#46377 about the same error messages, but different scenario. It has some context around what's being built/checked here. I haven't quite processed what it means for us yet.

For now I think we should drop MICROSOFT_BUILD_INFO in GOROOT with the info we want. (Easy enough to get with cat $(go env GOROOT)/MICROSOFT_BUILD_INFO or platform equivalent.)

@jaredpar
Copy link
Member

jaredpar commented Oct 5, 2021

Wow, that's quite surprising. I would not have expected it to have semantic meaning for the build. Assuming it was just for developer benefit.

Yes I agree for now we need to go with the simpler MICROSOFT_BUILD_INFO file.

@qmuntal
Copy link
Member

qmuntal commented Nov 4, 2021

I've created a specific issue #262 to discuss about go version after talking with @dagood. It also has a proposal to modify the reported version without changing upstream code.

@dagood
Copy link
Member Author

dagood commented Jun 7, 2023

Seeing this again later... an ergonomic reason to do this would be to make our aka.ms link
https://aka.ms/golang/release/latest/go1.20.src.tar.gz.sig
put
go.1.20.5-1.src.tar.gz.sig
rather than
go.20230606.4.src.tar.gz.sig
into your downloads folder.

If we do this, we should include the revision (-1) so the filename is unique per release build. This avoids human mixups, but also lets toolset downloader tools assume that each filename only needs to be downloaded once when building up a local cache.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants