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

bump go-cs3apis to latest master #4759

Merged
merged 4 commits into from
Jul 11, 2024
Merged

bump go-cs3apis to latest master #4759

merged 4 commits into from
Jul 11, 2024

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented Jul 9, 2024

This needed to touch quite a few unit test since the tooling to generate the go bindings was updated to a more recent version. Which causes issues with e.g. using reflect.DeepEqual on proto messages. We're basically running into the changes described here: https://github.com/protocolbuffers/protobuf-go/releases/tag/v1.20.0#v1.20-generated-code

@rhafer rhafer self-assigned this Jul 9, 2024
@rhafer rhafer requested review from a team, labkode and glpatcern as code owners July 9, 2024 14:55
@rhafer rhafer marked this pull request as draft July 9, 2024 14:56
@rhafer
Copy link
Contributor Author

rhafer commented Jul 9, 2024

Keeping this as a draft for now as I need to test it against ocis.

@rhafer rhafer force-pushed the ocisissue/9554 branch 9 times, most recently from bc96029 to 7d9dd17 Compare July 11, 2024 07:04
This contains the change that decouples the spaces API from storage API.
The tooling to generate	the go bindings has been updated. This cleans up
the fallout of that change (e.g. that protobuf messages can no longer be
compared reliably with reflect.DeepEqual).
protobuf messages are not supposed to be copied. This changes
the code to use pointers instead. Or proto.Clone() if a copy is
really needed.

Addresss ("copylocks: xxxx passes lock by value ...") issues raised by go-vet
@rhafer
Copy link
Contributor Author

rhafer commented Jul 11, 2024

There's a draft PR testing this in ocis: owncloud/ocis#9587

@rhafer rhafer marked this pull request as ready for review July 11, 2024 13:35
@rhafer rhafer marked this pull request as draft July 11, 2024 13:38
Copy link
Member

@micbar micbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heroic! 🎉

@glpatcern next time it would be helpful to upgrade the proto tooling in a separate change because that caused a lot of fallout.

@rhafer rhafer marked this pull request as ready for review July 11, 2024 14:37
@rhafer rhafer assigned micbar and unassigned rhafer Jul 11, 2024
@micbar micbar merged commit f9083e9 into cs3org:edge Jul 11, 2024
9 of 10 checks passed
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 this pull request may close these issues.

2 participants