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

Test data must be on 'testdata' directory not to break bazel / gazelle #102

Closed
FranciscoKurpiel opened this issue Apr 17, 2023 · 15 comments
Closed

Comments

@FranciscoKurpiel
Copy link

FranciscoKurpiel commented Apr 17, 2023

Rename internal/v2/tlsconfigstore/example_cert_key to internal/v2/tlsconfigstore/testdata to avoid breaking bazel users. Make sure this project can be used by bazel users with gazelle out-of-the-box, without the need for workarounds.

Context

Projects that use bazel will frequently also use gazelle to generate BUILD.bazel files. Since bazel can also run tests it needs to know what files are accessed by tests for hermicity reasons, allowing tests to be rerun when data changes. Gazelle automatically declares test data for all files inside directories that are named testdata. If a test uses files from outside of testdata directory, bazel will fail to analyze the build files with a build error. Ideally code that depends on this project should be usable out-of-the-box by bazel users with gazelle.

Workarounds are possible, but requiring a workaround will make using anything that depends on this project annoying to use.

Minimally reproducible example

Project: https://gitlab.com/xyko/s2a-go-bazelisk-issue

Step-by-step

Please have bazelisk installed, then:

git clone https://gitlab.com/xyko/s2a-go-bazelisk-issue
cd s2a-go-bazelisk-issue
go mod vendor
bazel run //:gazelle
bazel test //...

Expected

Successful run.

Observed

ERROR: /home/xyko/tmp/s2a-go-bazelisk-issue/vendor/github.com/google/s2a-go/internal/v2/BUILD:41:8: no such package 'internal/proto/common_go_proto': BUILD file not found in any of the following directories. Add a BUILD file to a directory to mark it as a package.
 - /home/xyko/tmp/s2a-go-bazelisk-issue/internal/proto/common_go_proto and referenced by '//vendor/github.com/google/s2a-go/internal/v2:v2_test'
ERROR: /home/xyko/tmp/s2a-go-bazelisk-issue/vendor/github.com/google/s2a-go/internal/v2/BUILD:41:8: no such package 'internal/v2/tlsconfigstore/example_cert_key': BUILD file not found in any of the following directories. Add a BUILD file to a directory to mark it as a package.
 - /home/xyko/tmp/s2a-go-bazelisk-issue/internal/v2/tlsconfigstore/example_cert_key and referenced by '//vendor/github.com/google/s2a-go/internal/v2:v2_test'
ERROR: Analysis of target '//vendor/github.com/google/s2a-go/internal/v2:v2_test' failed; build aborted: Analysis failed
INFO: Elapsed time: 0.244s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded, 31 targets configured)
ERROR: Couldn't start the build. Unable to run tests

--edit: fixed steps to reproduce the issue and the error message. The previous steps missed the vendoring step, and the result error message was reflecting that, instead of showing the issue I was trying to show. Sorry for the confusion.

@xmenxk
Copy link
Collaborator

xmenxk commented Apr 17, 2023

Thanks for reporting!

The error msg doesn't seem to suggest issue with the directory naming convention.
You might want to run the gazelle update-repos command though: https://github.com/bazelbuild/bazel-gazelle#update-repos

@FranciscoKurpiel
Copy link
Author

Now I feel silly :). Up to now I've being only invoking bazel run //:gazelle. Using bazel run //:gazelle update-repos -- -from_file=go.mod -prune=true did the trick. Thanks for the quick response.

@FranciscoKurpiel
Copy link
Author

Sorry for reverting myself, but but simply invoking update-repos doesn't actually help. I don't know how to get his package to work with bazel.

@psalaberria002
Copy link

psalaberria002 commented Apr 18, 2023

I am also struggling importing this repository with Gazelle. Especially when working with org_golang_google_api which needs access to s2a, but the go_library target @com_github_google_s2a_go//:s2a isn't visible to other packages.

This is what I tried

    go_repository(
        name = "org_golang_google_api",
        build_file_proto_mode = "disable_global",
        build_directives = [
            "gazelle:resolve go github.com/google/s2a-go @com_github_google_s2a_go//:s2a",
            "gazelle:resolve go github.com/google/s2a-go/fallback @com_github_google_s2a_go//fallback:s2a_fallback",
        ],
        importpath = "google.golang.org/api",
        sum = "h1:FNfHq9Z2GKULxu7cEhCaB0wWQHg43UpomrrN+24ZRdE=",
        version = "v0.118.0",
    )
    go_repository(
        name = "com_github_google_s2a_go",
        build_file_generation = "auto",
        build_file_proto_mode = "disable_global",
        importpath = "github.com/google/s2a-go",
        sum = "h1:3Qm0liEiCErViKERO2Su5wp+9PfMRiuS6XB5FvpKnYQ=",
        version = "v0.1.0",
    )

I have also tried to build with build_file_generation set to on, and updating the build_directives to the default gazelle targets, but in that case Gazelle is not creating the build files as expected. I get the following error.

ERROR: /home/paulsmsm/.cache/bazel/_bazel_paulsmsm/88642abe583907c56853b2008dde43be/external/com_github_google_s2a_go/internal/v2/tlsconfigstore/BUILD.bazel:27:8: Label '@com_github_google_s2a_go//internal/v2/tlsconfigstore:example_cert_key/server_cert.pem' is invalid because '@com_github_google_s2a_go//internal/v2/tlsconfigstore/example_cert_key' is a subpackage; perhaps you meant to put the colon here: '@com_github_google_s2a_go//internal/v2/tlsconfigstore/example_cert_key:server_cert.pem'?

Seems like a Gazelle issue. Not sure how to work around it.

@xmenxk
Copy link
Collaborator

xmenxk commented Apr 18, 2023

We did fix a bazel target naming issue in v0.1.1, so if you try that in your go.mod file, it should work.
I've also updated @org_golang_google_api 's dependency, pending a new release. sry for the inconvenience, hope this helps.

@FranciscoKurpiel
Copy link
Author

The project I created to reproduce the issue was already using v0.1.1.

I also faced the issue after org_golang_google_api added this project as a dependnecy, but this minimally reproducible project only include this project. I don't know if the google api repo has an issue, but to me it looks clear that this one has its own problem with gazelle.

@xmenxk
Copy link
Collaborator

xmenxk commented Apr 18, 2023

@FranciscoKurpiel do you mind share the errors messages with the v0.1.1 release? thanks

@FranciscoKurpiel
Copy link
Author

FranciscoKurpiel commented Apr 18, 2023

Gladly. The original post was with v0.1.1, so is the same error:

git clone https://gitlab.com/xyko/s2a-go-bazelisk-issue
cd s2a-go-bazelisk-issue
go mod vendor
bazel run //:gazelle update-repos -- -from_file=go.mod -prune=true # this line makes no difference
bazel run //:gazelle
bazel test //...
# Output:
# ERROR: /home/xyko/tmp/s2a-go-bazelisk-issue/vendor/github.com/google/s2a-go/internal/v2/BUILD:41:8: no such package 'internal/proto/common_go_proto': BUILD file not found in any of the following directories. Add a BUILD file to a directory to mark it as a package.
#  - /home/xyko/tmp/s2a-go-bazelisk-issue/internal/proto/common_go_proto and referenced by '//vendor/github.com/google/s2a-go/internal/v2:v2_test'
# ERROR: /home/xyko/tmp/s2a-go-bazelisk-issue/vendor/github.com/google/s2a-go/internal/v2/BUILD:41:8: no such package 'internal/v2/tlsconfigstore/example_cert_key': BUILD file not found in any of the following directories. Add a BUILD file to a directory to mark it as a package.
#  - /home/xyko/tmp/s2a-go-bazelisk-issue/internal/v2/tlsconfigstore/example_cert_key and referenced by '//vendor/github.com/google/s2a-go/internal/v2:v2_test'
# ERROR: Analysis of target '//vendor/github.com/google/s2a-go/internal/v2:v2_test' failed; build aborted: Analysis failed
# INFO: Elapsed time: 0.244s
# INFO: 0 processes.
# FAILED: Build did NOT complete successfully (0 packages loaded, 31 targets configured)
# ERROR: Couldn't start the build. Unable to run tests

@xmenxk
Copy link
Collaborator

xmenxk commented Apr 18, 2023

@FranciscoKurpiel please take a look at #99 (comment) for a potential short-term fix.
I'll come back to this when have a better solution. thanks

@noahdietz
Copy link

fwiw @xmenxk when utilizing the build_file_name attribute for the s2a-go go_repository to set BUILD.bazel and force file gen, I got the following warnings/errors:

DEBUG: .../external/bazel_gazelle/internal/go_repository.bzl:209:18: com_github_google_s2a_go: gazelle: .../external/com_github_google_s2a_go/internal/v2/tlsconfigstore/tlsconfigstore_test.go:53:3: pattern example_cert_key/client_cert.pem: matched no files
...
ERROR: .../external/com_github_google_s2a_go/internal/v2/remotesigner/BUILD.bazel:22:8: Label '@com_github_google_s2a_go//internal/v2/remotesigner:example_cert_key/client_cert.der' is invalid because '@com_github_google_s2a_go//internal/v2/remotesigner/example_cert_key' is a subpackage; perhaps you meant to put the colon here: '@com_github_google_s2a_go//internal/v2/remotesigner/example_cert_key:client_cert.der'?

Could be an issue in how I set it up, but that looks like an issue in how gazelle handles the testdata files. In the meantime, I've reverted my project (Go client library generation for googleapis) to use google.golang.org/api@v0.116.0 to unblock things.

@xmenxk
Copy link
Collaborator

xmenxk commented Apr 18, 2023

Thanks @noahdietz I'm working two things to address the issue: 1. remove existing bazel BUILD files from the s2a repo, and 2. make sure gazelle can properly generate BUILD.bazel files for the repo.

will keep you updated.

@FranciscoKurpiel
Copy link
Author

@FranciscoKurpiel please take a look at #99 (comment) for a potential short-term fix. I'll come back to this when have a better solution. thanks

Removing preexisting BUILD files fixes the issue.

I noticed the original reported reproduction steps was missing one step, and the error message on the description of this issue were both incorrect. I was totally blind to it up to now. Sorry for it.

@xmenxk
Copy link
Collaborator

xmenxk commented Apr 19, 2023

@FranciscoKurpiel @psalaberria002 created a new release v0.1.2, which should work with bazel/gazelle now

@noahdietz
Copy link

The updated dependency will be present in the upcoming google.golang.org/api release v0.119.0 for those curious. I tested it locally and it seems to have resolved the issues. Thanks @xmenxk great job.

@FranciscoKurpiel
Copy link
Author

v0.1.2 works perfectly on my side. Thanks for the super quick fix.

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

4 participants