Use git for detecting a dirty worktree.
#1319
Conversation
The `go-git` implementation of `git status` is outrageously expensive, as it performs a hash-based comparision of the working tree against the committed state with no caching. In some example runs, this takes upwards of 15 seconds. Because this is on the startup path for updates, this results in a rather poor user experience. These changes replace the `go-git` implementation with a call to `git status --porcelain -z`, which only writes data to stdout if the working tree is dirty.
| *w = true | ||
| } | ||
| return len(d), nil | ||
| } |
There was a problem hiding this comment.
Fun. Would not have thought of this approach :-).
cmd/util.go
Outdated
| if err != nil { | ||
| return m, errors.Wrapf(err, gitErrCtx, "getting worktree status") | ||
| // nolint: gas | ||
| gitStatusCmd := exec.Command(gitBin, "status", "--porcelain", "-z") |
There was a problem hiding this comment.
My one worry is that it is possible there will be non-zero environments where pulumi may be used where git is not available.
Do we have to hard fail on git not being installed? Is there a reasonable default value (and perhaps a warning?)
/cc @chrsmith
There was a problem hiding this comment.
We could fall back to the go-git implementation. We could also simply not set the property in order to indicate a lack of information.
There was a problem hiding this comment.
We could also simply not set the property in order to indicate a lack of information.
FWIW, I prefer this approach for all update metadata.
There was a problem hiding this comment.
We try hard not to require any data about a stack in the service. So simply not including the data won't cause any problems.
That having been said, the more data we can push into the service, the more linking and cross referencing we can do. So as a general principle we should try to have a fallback where appropriate.
Though it seems pretty reasonable to not include git data if git isn't on your PATH.
cmd/util.go
Outdated
| if ee, ok := err.(*exec.ExitError); ok { | ||
| ee.Stderr = stderr.Bytes() | ||
| } | ||
| return m, errors.Wrapf(err, gitErrCtx, fmt.Sprintf("invoking git status")) |
|
(it will certainly help substantially with #1150, and may even fix that issue) |
cmd/util.go
Outdated
| @@ -355,6 +357,15 @@ func (cf *colorFlag) Colorization() colors.Colorization { | |||
| return cf.value | |||
| } | |||
|
|
|||
| type anyWriter bool | |||
There was a problem hiding this comment.
Nit: A comment here would be nice, as non-level-9 Go masters may not recognize what you are up to.
cmd/util.go
Outdated
| // Commit at HEAD | ||
| head, err := repo.Head() | ||
| if err != nil { | ||
| glog.Warningf("getting HEAD commit") |
There was a problem hiding this comment.
Nit: Include err in the Warningf statement. Here and on line 431.
The
go-gitimplementation ofgit statusis outrageously expensive,as it performs a hash-based comparision of the working tree against the
committed state with no caching. In some example runs, this takes
upwards of 15 seconds. Because this is on the startup path for updates,
this results in a rather poor user experience.
These changes replace the
go-gitimplementation with a call togit status --porcelain -z, which only writes data to stdout if the workingtree is dirty.