Add build guards for all Windows imports#191
Add build guards for all Windows imports#191katiewasnothere merged 5 commits intomicrosoft:masterfrom uhthomas:master
Conversation
|
|
|
Note to maintainers: Should build guards be added to all files, regardless of imports? This repository is Windows specific, so it might make sense? |
|
Thanks! I just opened a similar PR before noticing there was one already (closed it now: #193) Do you think it's worth to consider adding a |
I'm not super sure how they're handled either. If the absence of
I think that's fine as well, especially since it makes any user errors more understandable. |
|
@katiewasnothere re: build guards for all files; I initially like the idea, but it seems odd to constrain files which have no platform-specific requirements. I'll leave it as is for now (protect windows imports). |
|
Hmm, looks like I might actually be missing a few build guards. Give me a moment. |
|
Okay, had to go through and add a bunch of build tags to files which implicitly use Windows-specific code (i.e tests). Go test seems happy ➜ go-winio git:(master) go test ./...
? github.com/Microsoft/go-winio [no test files]
? github.com/Microsoft/go-winio/backuptar [no test files]
? github.com/Microsoft/go-winio/pkg/etw [no test files]
? github.com/Microsoft/go-winio/wim/lzx [no test files]But Bazel does not ➜ wtest git:(master) ✗ bazel test @local_go_winio//...
Starting local Bazel server and connecting to it...
INFO: Analyzed 23 targets (88 packages loaded, 7439 targets configured).
INFO: Found 17 targets and 6 test targets...
ERROR: /home/thomas/.cache/bazel/_bazel_thomas/48d13cdd56897dee270c57999076ee09/external/local_go_winio/tools/etw-provider-gen/BUILD.bazel:16:10: GoLink external/local_go_winio/tools/etw-provider-gen/etw-provider-gen_/etw-provider-gen failed: (Exit 1): builder failed: error executing command bazel-out/host/bin/external/go_sdk/builder link -sdk external/go_sdk -installsuffix linux_amd64 -package_list bazel-out/host/bin/external/go_sdk/packages.txt -o ... (remaining 11 argument(s) skipped)
Use --sandbox_debug to see verbose messages from the sandbox builder failed: error executing command bazel-out/host/bin/external/go_sdk/builder link -sdk external/go_sdk -installsuffix linux_amd64 -package_list bazel-out/host/bin/external/go_sdk/packages.txt -o ... (remaining 11 argument(s) skipped)
Use --sandbox_debug to see verbose messages from the sandbox
external/go_sdk/pkg/tool/linux_amd64/link: bazel-out/k8-fastbuild/bin/external/local_go_winio/tools/etw-provider-gen/etw-provider-gen.a: not package main
link: error running subcommand external/go_sdk/pkg/tool/linux_amd64/link: exit status 2
INFO: Elapsed time: 9.307s, Critical Path: 1.48s
INFO: 96 processes: 26 internal, 70 linux-sandbox.
FAILED: Build did NOT complete successfully
@local_go_winio//:go-winio_test NO STATUS
@local_go_winio//backuptar:backuptar_test NO STATUS
@local_go_winio//pkg/etw:etw_test NO STATUS
@local_go_winio//pkg/etwlogrus:etwlogrus_test NO STATUS
@local_go_winio//pkg/guid:guid_test NO STATUS
@local_go_winio//pkg/security:security_test NO STATUS
FAILED: Build did NOT complete successfullyLooks like it's complaining about there being |
|
Bazel is now happy following the addition of noop mains for non Windows builds. ➜ wtest git:(master) ✗ bazel test @local_go_winio//...
INFO: Analyzed 23 targets (0 packages loaded, 0 targets configured).
INFO: Found 17 targets and 6 test targets...
INFO: Elapsed time: 0.475s, Critical Path: 0.34s
INFO: 24 processes: 1 internal, 23 linux-sandbox.
INFO: Build completed successfully, 24 total actions
@local_go_winio//:go-winio_test PASSED in 0.0s
@local_go_winio//backuptar:backuptar_test PASSED in 0.0s
@local_go_winio//pkg/etw:etw_test PASSED in 0.0s
@local_go_winio//pkg/etwlogrus:etwlogrus_test PASSED in 0.0s
@local_go_winio//pkg/guid:guid_test PASSED in 0.0s
@local_go_winio//pkg/security:security_test PASSED in 0.0s
Executed 6 out of 6 tests: 6 tests pass.
There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones INFO: Build completed successfully, 24 total actionsand just to double check, so is Go test. ➜ go-winio git:(master) go test ./...
? github.com/Microsoft/go-winio [no test files]
? github.com/Microsoft/go-winio/backuptar [no test files]
? github.com/Microsoft/go-winio/pkg/etw [no test files]
? github.com/Microsoft/go-winio/pkg/etw/sample [no test files]
? github.com/Microsoft/go-winio/tools/etw-provider-gen [no test files]
? github.com/Microsoft/go-winio/wim/lzx [no test files]
? github.com/Microsoft/go-winio/wim/validate [no test files]Do these changes look okay to you? |
Fixes #190