Dep: Formating arrays in Gopkg.lock makes it undiffable

[tnozicka]

What version of Go (go version) and dep (git describe --tags) are you using?

go version go1.8.3 linux/amd64
dep - v0.3.0-22-g11758a7

This is part of my Gopkg.lock file. If I ever change one of these packages that will be one ugly diff :/ I am not sure if this is a limitation of TOML or dep just doesn't apply formatting there but did you consider doing one package per line?

[[projects]]
 +  name = "k8s.io/client-go"
 +  packages = ["discovery","kubernetes","kubernetes/typed/apps/v1beta1","kubernetes/typed/authentication/v1beta1","kubernetes/typed/authorization/v1beta1","kubernetes/typed/autoscaling/v1","kubernetes/typed/batch/v1","kubernetes/typed/batch/v2alpha1","kubernetes/typed/certificates/v1alpha1","kubernetes/typed/core/v1","kubernetes/typed/extensions/v1beta1","kubernetes/typed/policy/v1beta1","kubernetes/typed/rbac/v1alpha1","kubernetes/typed/storage/v1beta1","pkg/api","pkg/api/errors","pkg/api/install","pkg/api/meta","pkg/api/meta/metatypes","pkg/api/resource","pkg/api/unversioned","pkg/api/v1","pkg/api/validation/path","pkg/apimachinery","pkg/apimachinery/announced","pkg/apimachinery/registered","pkg/apis/apps","pkg/apis/apps/install","pkg/apis/apps/v1beta1","pkg/apis/authentication","pkg/apis/authentication/install","pkg/apis/authentication/v1beta1","pkg/apis/authorization","pkg/apis/authorization/install","pkg/apis/authorization/v1beta1","pkg/apis/autoscaling","pkg/apis/autoscaling/install","pkg/apis/autoscaling/v1","pkg/apis/batch","pkg/apis/batch/install","pkg/apis/batch/v1","pkg/apis/batch/v2alpha1","pkg/apis/certificates","pkg/apis/certificates/install","pkg/apis/certificates/v1alpha1","pkg/apis/extensions","pkg/apis/extensions/install","pkg/apis/extensions/v1beta1","pkg/apis/policy","pkg/apis/policy/install","pkg/apis/policy/v1beta1","pkg/apis/rbac","pkg/apis/rbac/install","pkg/apis/rbac/v1alpha1","pkg/apis/storage","pkg/apis/storage/install","pkg/apis/storage/v1beta1","pkg/auth/user","pkg/conversion","pkg/conversion/queryparams","pkg/fields","pkg/genericapiserver/openapi/common","pkg/labels","pkg/runtime","pkg/runtime/serializer","pkg/runtime/serializer/json","pkg/runtime/serializer/protobuf","pkg/runtime/serializer/recognizer","pkg/runtime/serializer/streaming","pkg/runtime/serializer/versioning","pkg/selection","pkg/third_party/forked/golang/reflect","pkg/third_party/forked/golang/template","pkg/types","pkg/util","pkg/util/cert","pkg/util/clock","pkg/util/errors","pkg/util/flowcontrol","pkg/util/framer","pkg/util/homedir","pkg/util/integer","pkg/util/intstr","pkg/util/json","pkg/util/jsonpath","pkg/util/labels","pkg/util/net","pkg/util/parsers","pkg/util/rand","pkg/util/runtime","pkg/util/sets","pkg/util/uuid","pkg/util/validation","pkg/util/validation/field","pkg/util/wait","pkg/util/yaml","pkg/version","pkg/watch","pkg/watch/versioned","plugin/pkg/client/auth","plugin/pkg/client/auth/gcp","plugin/pkg/client/auth/oidc","rest","tools/auth","tools/clientcmd","tools/clientcmd/api","tools/clientcmd/api/latest","tools/clientcmd/api/v1","tools/metrics","transport"]
 +  revision = "e121606b0d09b2e1c467183ee46217fa85a6b672"
 +  version = "v2.0.0"

[sdboyer]

mmm yeah, this is a very good point. we quite intentionally sort those packages in order to produce diffable output, but don't output them one per line, which makes the sorting pointless.

@pelletier? @carolynvs?

(in the meantime, git --word-diff is your friend)

[carolynvs]

The TOML lib doesn't have any settings so that you can tweak the output formatting yet. I think if we wanted to add something like this (and I agree the current formatting is hard to read, let alone diff), the place to start looking would be herey

[carolynvs]

The TOML lib doesn't have any settings so that you can tweak the output formatting yet. I think if we wanted to add something like this (and I agree the current formatting is hard to read, let alone diff), the place to start looking would be here.

[pelletier]

Adding formatting options seems like a good idea to me. Seems like you are using toml.Marshal, I just wouldn't add the options to this method (to keep it consistent with other uses of Marshal in go), but maybe a MarshalWithOptions that takes a struct that defines a few formatting options?

[ydnar]

This week we started using dep in production, and set up automatic canary builds to watch for updates to our app’s dependencies. We have a job that runs nightly on CircleCI, runs dep ensure -update, tests, and commits the changes to Gopkg.lock and submits a pull request on GitHub. This lets us manually review the changes and test output before merging to master.

It’s great, with one exception: the diffs for Gopkg.lock are less than useful for the reasons outlined in this issue (and dep doesn’t produce meaningful output on what changed).

The logical fix (to us, anyway) would be to sort the packages key in the TOML output last, after name, revision, and version. That way git diff will show enough context by default to see which package(s) changed.

OK if we submit a pull request that implements this?

[ChrisHines]

I thought this was already fixed dep in 0.4.x. Merged here: https://github.com/golang/dep/pull/1461

[ydnar]

Ah, maybe what we have is a subtly different issue, perhaps created by the fix for this bug (multi-line arrays).

The packages array can be several lines long, pushing version and revision away from the package name and other keys.

[JFixby]

+