Skip to content

Commit

Permalink
[REFACTORING] Add versioned go binary rules to eliminate duplicate code
Browse files Browse the repository at this point in the history
Summary: Also add testonly to avoid misuse

Test Plan: Refactoring, use existing tests

Reviewers: #stirling, jamesbartlett

Reviewed By: jamesbartlett

Signed-off-by: yzhao1012 <[email protected]>

Differential Revision: https://phab.corp.pixielabs.ai/D11622

GitOrigin-RevId: 10d0cd6
  • Loading branch information
yzhao1012 authored and copybaranaut committed Jun 17, 2022
1 parent 3e26323 commit 1c388a1
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 33 deletions.
8 changes: 4 additions & 4 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,11 @@ gazelle_dependencies(go_sdk = "go_sdk")

# Download alternative go toolchains after all other dependencies, so that they aren't used by external dependencies.
go_download_sdk(
name = "go_sdk_1_17",
version = "1.17.11",
name = "go_sdk_1_16",
version = "1.16.14",
)

go_download_sdk(
name = "go_sdk_1_16",
version = "1.16.14",
name = "go_sdk_1_17",
version = "1.17.11",
)
8 changes: 4 additions & 4 deletions src/stirling/obj_tools/dwarf_reader_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ constexpr std::string_view kTestGo1_16Binary =
constexpr std::string_view kTestGo1_17Binary =
"src/stirling/obj_tools/testdata/go/test_go_1_17_binary_/"
"test_go_1_17_binary";
constexpr std::string_view kTestGo1_18Binary =
"src/stirling/obj_tools/testdata/go/test_go_1_18_binary_/"
"test_go_1_18_binary";
constexpr std::string_view kTestGoBinary =
"src/stirling/obj_tools/testdata/go/test_go_binary_/"
"test_go_binary";
constexpr std::string_view kGoGRPCServer =
"src/stirling/testing/demo_apps/go_grpc_tls_pl/server/golang_1_16_grpc_tls_server_binary_/"
"golang_1_16_grpc_tls_server_binary";
Expand Down Expand Up @@ -70,7 +70,7 @@ class DwarfReaderTest : public ::testing::TestWithParam<DwarfReaderTestParam> {
: kCppBinaryPath(px::testing::BazelRunfilePath(kCppBinary)),
kGo1_16BinaryPath(px::testing::BazelRunfilePath(kTestGo1_16Binary)),
kGo1_17BinaryPath(px::testing::BazelRunfilePath(kTestGo1_17Binary)),
kGo1_18BinaryPath(px::testing::BazelRunfilePath(kTestGo1_18Binary)),
kGo1_18BinaryPath(px::testing::BazelRunfilePath(kTestGoBinary)),
kGoServerBinaryPath(px::testing::BazelRunfilePath(kGoGRPCServer)),
kGoBinaryUnconventionalPath(px::testing::BazelRunfilePath(kGoBinaryUnconventional)) {}

Expand Down
31 changes: 6 additions & 25 deletions src/stirling/obj_tools/testdata/go/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# SPDX-License-Identifier: Apache-2.0

load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
load("//src/stirling/testing:go_versioned_binary.bzl", "go_1_16_binary", "go_1_17_binary")

package(default_visibility = ["//src/stirling:__subpackages__"])

Expand All @@ -24,39 +25,18 @@ go_library(
importpath = "px.dev/pixie/src/stirling/obj_tools/testdata/go",
)

go_binary(
go_1_16_binary(
name = "test_go_1_16_binary",
embed = [":lib"],
# -N -l are set by default in debug builds, and go tool compile re-enables inlining if it sees `-l -l`.
gc_goopts = select({
"//bazel:debug_build": [],
"//conditions:default": [
"-N",
"-l",
],
}),
goarch = "amd64",
goos = "linux",
gosdk = "@go_sdk_1_16//:go_sdk",
)

go_binary(
go_1_17_binary(
name = "test_go_1_17_binary",
embed = [":lib"],
gc_goopts = select({
"//bazel:debug_build": [],
"//conditions:default": [
"-N",
"-l",
],
}),
goarch = "amd64",
goos = "linux",
gosdk = "@go_sdk_1_17//:go_sdk",
)

go_binary(
name = "test_go_1_18_binary",
name = "test_go_binary",
embed = [":lib"],
gc_goopts = select({
"//bazel:debug_build": [],
Expand All @@ -69,10 +49,11 @@ go_binary(

filegroup(
name = "test_binaries",
testonly = True,
srcs = [
"sockshop_payments_service",
":test_go_1_16_binary",
":test_go_1_17_binary",
":test_go_1_18_binary",
":test_go_binary",
],
)
48 changes: 48 additions & 0 deletions src/stirling/testing/go_versioned_binary.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Copyright 2018- The Pixie Authors.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
# SPDX-License-Identifier: Apache-2.0

load("@io_bazel_rules_go//go:def.bzl", "go_binary")

# Allow building a go binary with specified go SDK. Used to verify Pixie's compatibility with
# different golang SDK versions. For example, verify Pixie can handle different go ABIs.
def _go_versioned_binary_impl(name, embed, sdk):
go_binary(
name = name,
embed = embed,
# -N -l are set by default in debug builds, and go tool compile re-enables inlining
# if it sees `-l -l`, i.e., options are toggled each time it's specified.
# According to go tool compile -help
# -N disable optimizations
# -l disable inlining
# This makes sure the function symbol address stable across builds.
gc_goopts = select({
"//bazel:debug_build": [],
"//conditions:default": [
"-N",
"-l",
],
}),
testonly = True,
goarch = "amd64",
goos = "linux",
gosdk = sdk,
)

def go_1_16_binary(name, embed):
_go_versioned_binary_impl(name, embed, "@go_sdk_1_16//:go_sdk")

def go_1_17_binary(name, embed):
_go_versioned_binary_impl(name, embed, "@go_sdk_1_17//:go_sdk")

0 comments on commit 1c388a1

Please sign in to comment.