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

Unexpected OutputFiles Modification Due to Shallow Copy in Command#ToREProto() #610

Open
alsu4r opened this issue Dec 30, 2024 · 1 comment

Comments

@alsu4r
Copy link
Contributor

alsu4r commented Dec 30, 2024

I am using ReClient (https://github.com/bazelbuild/reclient) as the client and an RBE Service that supports RE API v2.1 or higher for Android RBE builds. During the build process, I encountered an issue where OutputFiles were unexpectedly modified. While debugging, I identified that this issue was caused by a shallow copy resulting from the use of append() in the Command#ToREProto() function in the remote-apis-sdk. Since Go's append can result in a shallow copy depending on the slice's capacity, shouldn't it be explicitly modified to ensure a deep copy?

I believe modifying the remote-apis-sdk code is a more appropriate solution to this issue than addressing it in ReClient, so I made adjustments to the SDK code. Below is the modification I tested.

There are many ways to address this issue, but this is how I resolved it: https://github.com/bazelbuild/remote-apis-sdks/blob/master/go/pkg/command/command.go#L600

  •    cmdPb.OutputPaths = append(append([]string{}, c.OutputFiles...), c.OutputDirs...)
    
@mrahs
Copy link
Collaborator

mrahs commented Jan 6, 2025

Good catch. The conversion must produce a deep clone. I'd allocate a properly sized slice before filling it up from both sources to avoid potential resizes. Please feel free to post a PR for the remote-apis-sdks.

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

No branches or pull requests

2 participants