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

Make go_test() with embedsrcs work #2936

Merged

Conversation

EdSchouten
Copy link
Contributor

@EdSchouten EdSchouten commented Aug 10, 2021

Even though there already is testing coverage for go_test() with
embedsrcs using generated files and such, I can't seem to get one of the
most trivial cases to work. The file to be embedded doesn't get placed
in the sandbox during GoCompilePkg.

This change addresses this by explicitly copying the embedsrcs from the
internal source to the external, which seems to make things work
properly.

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

It fixes go_test() with embedsrcs. It's needed, because I want to use that feature.

Which issues(s) does this PR fix?

I couldn't find any existing issues.

Other notes for review

Even though there already is testing coverage for go_test() with
embedsrcs using generated files and such, I can't seem to get one of the
most trivial cases to work. The file to be embedded doesn't get placed
in the sandbox during GoCompilePkg.

This change addresses this by explicitly copying the embedsrcs from the
internal source to the external, which seems to make things work
properly.
@google-cla google-cla bot added the cla: yes label Aug 10, 2021
@robfig
Copy link
Contributor

robfig commented Sep 23, 2021

It's surprising that this wasn't caught by the existing embedsrcs_test, which also embeds files outside the directory, but I've confirmed that fact locally. Thanks for the contribution, merging.

@robfig robfig merged commit 9f77676 into bazel-contrib:master Sep 23, 2021
@EdSchouten EdSchouten deleted the eschouten/20210810-embedsrcs branch September 25, 2021 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants