Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Formating arrays in Gopkg.lock makes it undiffable #972

Closed
tnozicka opened this issue Aug 8, 2017 · 8 comments
Closed

Formating arrays in Gopkg.lock makes it undiffable #972

tnozicka opened this issue Aug 8, 2017 · 8 comments

Comments

@tnozicka
Copy link

tnozicka commented Aug 8, 2017

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
Copy link
Member

sdboyer commented Aug 8, 2017

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
Copy link
Collaborator

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
Copy link
Collaborator

carolynvs commented Aug 8, 2017

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
Copy link

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
Copy link

ydnar commented Feb 16, 2018

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
Copy link
Contributor

ChrisHines commented Feb 16, 2018

I thought this was already fixed dep in 0.4.x. Merged here: #1461

@ydnar
Copy link

ydnar commented Feb 16, 2018

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
Copy link

JFixby commented May 28, 2018

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

No branches or pull requests

8 participants