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.

Sign up for GitHub

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

Jump to bottom

exp/cmd/apidiff: incorrectly reporting changes #34849

Closed
jeanbza opened this issue Oct 11, 2019 · 7 comments
Closed

exp/cmd/apidiff: incorrectly reporting changes #34849

jeanbza opened this issue Oct 11, 2019 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.14

Comments

@jeanbza
Copy link
Contributor

jeanbza commented Oct 11, 2019 •
edited
Loading

Running apidiff against two of the same thing results in changes. This must be impossible, since nothing has changed since apidiff -w and apidiff. This started happening within the last week, IIRC. Here's a repro:

deklerk at deklerk-macbookpro2 in ~/workspace/google-cloud-go/bigquery on rege*
$ cd /tmp
deklerk at deklerk-macbookpro2 in /tmp
$ git clone https://code.googlesource.com/gocloud gocloud1
Cloning into 'gocloud1'...
remote: Sending approximately 25.35 MiB ...
remote: Counting objects: 10, done
remote: Total 28180 (delta 15088), reused 28180 (delta 15088)
Receiving objects: 100% (28180/28180), 25.35 MiB | 8.26 MiB/s, done.
Resolving deltas: 100% (15088/15088), done.
deklerk at deklerk-macbookpro2 in /tmp
$ cd gocloud1/bigquery
deklerk at deklerk-macbookpro2 in /tmp/gocloud1/bigquery on master
$ apidiff -w /tmp/pkg.master cloud.google.com/go/bigquery
deklerk at deklerk-macbookpro2 in /tmp/gocloud1/bigquery on master
$ apidiff /tmp/pkg.master cloud.google.com/go/bigquery
Incompatible changes:
- Reader: removed
Compatible changes:
- AccessEntry: added
- AccessRole: added
- Avro: added
- BatchPriority: added
- Bigtable: added
- BigtableColumn: added
- BigtableColumnFamily: added
- BigtableOptions: added
- BooleanFieldType: added
- BytesFieldType: added
- CSV: added
- CSVOptions: added
- CivilDateTimeString: added
- CivilTimeString: added
- Client: added
- Clustering: added
- Compression: added
- Copier: added
- CopyConfig: added
- CreateIfNeeded: added
- CreateNever: added
- DataFormat: added
- Dataset: added
- DatasetIterator: added
- DatasetMetadata: added
- DatasetMetadataToUpdate: added
- DatastoreBackup: added
- DateFieldType: added
- DateTimeFieldType: added
- DomainEntity: added
- Done: added
- Encoding: added
- EncryptionConfig: added
- EntityType: added
- Error: added
- ExplainQueryStage: added
- ExplainQueryStep: added
- ExternalData: added
- ExternalDataConfig: added
- ExternalDataConfigOptions: added
- ExternalTable: added
- ExtractConfig: added
- ExtractStatistics: added
- Extractor: added
- FieldSchema: added
- FieldType: added
- FileConfig: added
- FloatFieldType: added
- GCSReference: added
- GeographyFieldType: added
- GoogleSheets: added
- GoogleSheetsOptions: added
- GroupEmailEntity: added
- Gzip: added
- ISO_8859_1: added
- InferSchema: added
- Inserter: added
- IntegerFieldType: added
- InteractivePriority: added
- JSON: added
- Job: added
- JobConfig: added
- JobIDConfig: added
- JobIterator: added
- JobStatistics: added
- JobStatus: added
- LoadConfig: added
- LoadSource: added
- LoadStatistics: added
- Loader: added
- Model: added
- ModelIterator: added
- ModelMetadata: added
- ModelMetadataToUpdate: added
- MultiError: added
- NeverExpire: added
- NewClient: added
- NewGCSReference: added
- NewReaderSource: added
- None: added
- NullBool: added
- NullDate: added
- NullDateTime: added
- NullFloat64: added
- NullGeography: added
- NullInt64: added
- NullString: added
- NullTime: added
- NullTimestamp: added
- NumericFieldType: added
- NumericPrecisionDigits: added
- NumericScaleDigits: added
- NumericString: added
- ORC: added
- OwnerRole: added
- Parquet: added
- Pending: added
- PutMultiError: added
- Query: added
- QueryConfig: added
- QueryParameter: added
- QueryPriority: added
- QueryStatistics: added
- QueryTimelineSample: added
- ReaderRole: added
- ReaderSource: added
- RecordFieldType: added
- RegularTable: added
- Routine: added
- RoutineArgument: added
- RoutineIterator: added
- RoutineMetadata: added
- RoutineMetadataToUpdate: added
- RowInsertionError: added
- RowIterator: added
- Running: added
- Schema: added
- SchemaFromJSON: added
- Scope: added
- Seed: added
- SpecialGroupEntity: added
- StandardSQLDataType: added
- StandardSQLField: added
- StandardSQLStructType: added
- State: added
- StateUnspecified: added
- Statistics: added
- StreamingBuffer: added
- StringFieldType: added
- StructSaver: added
- Table: added
- TableCreateDisposition: added
- TableIterator: added
- TableMetadata: added
- TableMetadataToUpdate: added
- TableType: added
- TableWriteDisposition: added
- TimeFieldType: added
- TimePartitioning: added
- TimestampFieldType: added
- TrainingRun: added
- UTF_8: added
- Uploader: added
- UserEmailEntity: added
- Value: added
- ValueLoader: added
- ValueSaver: added
- ValuesSaver: added
- ViewEntity: added
- ViewTable: added
- WriteAppend: added
- WriteEmpty: added
- WriteTruncate: added
- WriterRole: added
@jeanbza
Copy link
Contributor Author

jeanbza commented Oct 11, 2019

cc @jba

@jeanbza
Copy link
Contributor Author

jeanbza commented Oct 11, 2019 •
edited
Loading

FYI we are using golang.org/x/tools v0.0.0-20191010201905-e5ffc44a6fee. It may be useful to build an apidiff version for easier debugging, as gopls has.

gopherbot pushed a commit to googleapis/google-cloud-go that referenced this issue Oct 11, 2019
@jeanbza
internal: skip apidiff until golang/go#34849 is fixed
435b467 
Change-Id: I1feb4ba90352c93b0a0beca6046c2821b23cdf02
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/46790
Reviewed-by: Noah Dietz <[email protected]>
gopherbot pushed a commit to googleapis/google-cloud-go that referenced this issue Oct 11, 2019
@jeanbza
all: regen
32f85c9 
BREAKING_CHANGE_ACCEPTABLE=golang/go#34849

Change-Id: I95327c2ade0cb584ad439636f5635030d07a9353
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/46672
Reviewed-by: kokoro <[email protected]>
Reviewed-by: Tyler Bui-Palsulich <[email protected]>
@jba
Copy link
Contributor

jba commented Oct 11, 2019

Wild. Will take a look.

@jeanbza
Copy link
Contributor Author

jeanbza commented Oct 11, 2019 •
edited
Loading

tyvm! FWIW that directory is its own module nested under cloud.google.com/go, so maybe there's some weird multi-module thing going on.

@jba
Copy link
Contributor

jba commented Oct 14, 2019

Could not reproduce.
First I did:

> git clone https://go.googlesource.com/exp go-exp
> cd go-exp
> go install ./cmd/apidiff

Then I followed what you did. I got no output from the apidiff call. Just to make sure it was looking in the right place, I changed something in bigquery.go and ran it again, and it produced the right output.

@jeanbza
Copy link
Contributor Author

jeanbza commented Oct 14, 2019

When I install apidiff the way you did, the errors went away. But, when I use go get -u golang.org/x/tools && go install golang.org/x/exp/cmd/apidiff, I see the error. Here's another repro: https://gist.github.com/jadekler/934ea1fdd5bb6d47e70de0cd389a16ce. And, another repro on a goobuntu machine: https://gist.github.com/jadekler/ef356b1a1dee2a798e2c422ebfd41610.

I wonder if I'm somehow not getting the latest version of apidiff with go get && go install...? Also, it's weird to me that go get -u golang.org/x/exp doesn't result in a golang.org/x/exp entry in my go.mod. Any idea what's up with that?

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/200997 mentions this issue: cmd/apidiff: store package path with export data

@katiehockman katiehockman added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 14, 2019
@katiehockman katiehockman added this to the Go1.14 milestone Oct 14, 2019
@golang golang locked and limited conversation to collaborators Oct 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.14
Development

No branches or pull requests
4 participants
@jeanbza @gopherbot @jba @katiehockman