mrproliu commented on code in PR #231:
URL: https://github.com/apache/skywalking-go/pull/231#discussion_r2339357746
##########
.github/workflows/e2e.yaml:
##########
@@ -30,6 +30,12 @@ jobs:
- uses: actions/checkout@v2
with:
submodules: true
+ - name: Install unzip
+ run: sudo apt-get update && sudo apt-get install -y unzip
+ - name: Check Go version
Review Comment:
The setup Go already define the go version, why there still needs to check
again?
##########
.github/workflows/e2e.yaml:
##########
@@ -30,6 +30,12 @@ jobs:
- uses: actions/checkout@v2
with:
submodules: true
+ - name: Install unzip
+ run: sudo apt-get update && sudo apt-get install -y unzip
Review Comment:
Is the unzip not define in the original runner? If exist, please remove this
install step.
##########
tools/protocols/pull-proto.sh:
##########
@@ -0,0 +1,142 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+set -e
+
+# -----------------------------
+# Configuration
+# -----------------------------
+export GOPROXY=https://goproxy.cn,direct
+PROTOC_VERSION=3.14.0
+export PATH="/home/runner/go/bin:$PATH"
+export GOPATH="${GOPATH:-$HOME/go}"
+BASEDIR=$(dirname "$0")
+TEMPDIR="$BASEDIR"/temp
+BINDIR="$TEMPDIR"/bin
+INCLUDE_DIR="$TEMPDIR"/include
+
+SUBMODULE_PATH="skywalking-data-collect-protocol"
+OUTPUT_BASE_DIR="../../protocols"
+
+mkdir -p "$OUTPUT_BASE_DIR"
+mkdir -p "$BINDIR"
+mkdir -p "$INCLUDE_DIR"
+
+# -----------------------------
+# Install protoc (non-root)
+# -----------------------------
+if [[ ! -f "$BINDIR"/protoc ]]; then
+ echo "Installing protoc locally..."
+ UNAME=$(uname -s)
+ if [[ "$UNAME" == "Linux" ]]; then
+ PROTOC_ZIP="protoc-${PROTOC_VERSION}-linux-x86_64.zip"
+ elif [[ "$UNAME" == "Darwin" ]]; then
+ PROTOC_ZIP="protoc-${PROTOC_VERSION}-osx-x86_64.zip"
+ elif [[ "$UNAME" == MINGW* ]] || [[ "$UNAME" == CYGWIN* ]]; then
+ PROTOC_ZIP="protoc-${PROTOC_VERSION}-win64.zip"
+ else
+ echo "Unsupported OS: $UNAME"
+ exit 1
+ fi
+
+ curl -sL
"https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/${PROTOC_ZIP}";
-o "$TEMPDIR/$PROTOC_ZIP"
+
+ # Extract to temp directory first
+ EXTRACT_DIR="$TEMPDIR/extract"
+ mkdir -p "$EXTRACT_DIR"
+ unzip -o "$TEMPDIR/$PROTOC_ZIP" -d "$EXTRACT_DIR"
+
+ # Copy protoc binary
+ if [[ -f "$EXTRACT_DIR/bin/protoc" ]]; then
+ cp "$EXTRACT_DIR/bin/protoc" "$BINDIR/protoc"
+ elif [[ -f "$EXTRACT_DIR/bin/protoc.exe" ]]; then
+ cp "$EXTRACT_DIR/bin/protoc.exe" "$BINDIR/protoc"
+ else
+ echo "Error: protoc binary not found in archive"
+ exit 1
+ fi
+
+ # Copy include files
+ if [[ -d "$EXTRACT_DIR/include" ]]; then
+ cp -r "$EXTRACT_DIR/include"/* "$INCLUDE_DIR/"
+ fi
+
+ chmod +x "$BINDIR/protoc"
+ rm -rf "$EXTRACT_DIR"
+ rm -f "$TEMPDIR/$PROTOC_ZIP"
+fi
+
+# -----------------------------
+# Export PATH for local protoc
+# -----------------------------
+export PATH="$BINDIR:$GOPATH/bin:$PATH"
+rm -f "$GOPATH/bin/protoc-gen-go" "$GOPATH/bin/protoc-gen-go-grpc"
+# -----------------------------
+# Install Go plugins (fixed versions)
+# -----------------------------
+echo "Current go version:"
+go version
+if ! command -v protoc-gen-go &>/dev/null; then
+ echo "Installing protoc-gen-go v1.26..."
+ GO111MODULE=on GOPROXY=https://goproxy.cn,direct
GOSUMDB=sum.golang.google.cn go install
google.golang.org/protobuf/cmd/protoc-gen-go@v1.26.0
+fi
+
+if ! command -v protoc-gen-go-grpc &>/dev/null; then
+ echo "Installing protoc-gen-go-grpc v1.1..."
+ GO111MODULE=on GOPROXY=https://goproxy.cn,direct
GOSUMDB=sum.golang.google.cn go install
google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.1.0
Review Comment:
Please remove these env, just do `go install`.
##########
.github/workflows/e2e.yaml:
##########
@@ -50,9 +56,20 @@ jobs:
- name: Kafka
path: test/e2e/case/kafka/e2e.yaml
steps:
- - uses: actions/checkout@v2
+ - name: Set up Go
Review Comment:
In the Job, the agent have already built, so don't needs to generate the
protocol again.
##########
.gitignore:
##########
@@ -8,6 +8,7 @@ coverage.txt
/test/e2e/dist/
/test/plugins/workspace
/test/plugins/dist/
+/tools/protocols/temp/
Review Comment:
I think the `protocols` should also no need to be uploaded into the git?
##########
.github/workflows/plugin-tests.yaml:
##########
@@ -148,14 +160,18 @@ jobs:
- uses: actions/checkout@v2
with:
submodules: true
- - uses: actions/download-artifact@v4
- with:
- name: test-tools-macos-12
- path: test/plugins/dist
- name: Set up Go 1.19
uses: actions/setup-go@v2
with:
go-version: 1.19
+ - name: Install unzip
Review Comment:
Same here, the agent already built.
##########
Makefile:
##########
@@ -57,6 +57,12 @@ deps:
linter:
$(GO_LINT) version || curl -sfL
https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh
-s -- -b $(GO_PATH)/bin v1.50.0
+##@ General
+
+.PHONY: generate-proto
+generate-proto: ## generate data collect proto
+ cd tools/protocols && ./pull-proto.sh
Review Comment:
Please rename the `pull-proto` to `generate`, and you don't need to use `cd`
command.
##########
tools/protocols/pull-proto.sh:
##########
@@ -0,0 +1,142 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+set -e
+
+# -----------------------------
+# Configuration
+# -----------------------------
+export GOPROXY=https://goproxy.cn,direct
+PROTOC_VERSION=3.14.0
+export PATH="/home/runner/go/bin:$PATH"
+export GOPATH="${GOPATH:-$HOME/go}"
+BASEDIR=$(dirname "$0")
+TEMPDIR="$BASEDIR"/temp
+BINDIR="$TEMPDIR"/bin
+INCLUDE_DIR="$TEMPDIR"/include
+
+SUBMODULE_PATH="skywalking-data-collect-protocol"
+OUTPUT_BASE_DIR="../../protocols"
Review Comment:
You should use the absolute directory. If the user executes this command in
another directory, the output directory is not expected.
##########
.github/workflows/plugin-tests.yaml:
##########
@@ -110,6 +114,14 @@ jobs:
- uses: actions/checkout@v2
with:
submodules: true
+ - name: Set up Go 1.19
Review Comment:
The agent binary is already built. Please remove unnecessary generated code.
##########
tools/protocols/pull-proto.sh:
##########
@@ -0,0 +1,142 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+set -e
+
+# -----------------------------
+# Configuration
+# -----------------------------
+export GOPROXY=https://goproxy.cn,direct
+PROTOC_VERSION=3.14.0
+export PATH="/home/runner/go/bin:$PATH"
+export GOPATH="${GOPATH:-$HOME/go}"
Review Comment:
Why do you need to declare `GOPROXY`, `PATH`, and `GOPATH`? It should use
the user system configuration.
User system doesn't have `/home/runner`, it should only work for the GitHub
Runner.
##########
tools/protocols/pull-proto.sh:
##########
@@ -0,0 +1,142 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+set -e
+
+# -----------------------------
+# Configuration
+# -----------------------------
+export GOPROXY=https://goproxy.cn,direct
+PROTOC_VERSION=3.14.0
+export PATH="/home/runner/go/bin:$PATH"
+export GOPATH="${GOPATH:-$HOME/go}"
+BASEDIR=$(dirname "$0")
+TEMPDIR="$BASEDIR"/temp
+BINDIR="$TEMPDIR"/bin
+INCLUDE_DIR="$TEMPDIR"/include
+
+SUBMODULE_PATH="skywalking-data-collect-protocol"
+OUTPUT_BASE_DIR="../../protocols"
+
+mkdir -p "$OUTPUT_BASE_DIR"
+mkdir -p "$BINDIR"
+mkdir -p "$INCLUDE_DIR"
+
+# -----------------------------
+# Install protoc (non-root)
+# -----------------------------
+if [[ ! -f "$BINDIR"/protoc ]]; then
+ echo "Installing protoc locally..."
+ UNAME=$(uname -s)
+ if [[ "$UNAME" == "Linux" ]]; then
+ PROTOC_ZIP="protoc-${PROTOC_VERSION}-linux-x86_64.zip"
+ elif [[ "$UNAME" == "Darwin" ]]; then
+ PROTOC_ZIP="protoc-${PROTOC_VERSION}-osx-x86_64.zip"
+ elif [[ "$UNAME" == MINGW* ]] || [[ "$UNAME" == CYGWIN* ]]; then
+ PROTOC_ZIP="protoc-${PROTOC_VERSION}-win64.zip"
+ else
+ echo "Unsupported OS: $UNAME"
+ exit 1
+ fi
+
+ curl -sL
"https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/${PROTOC_ZIP}";
-o "$TEMPDIR/$PROTOC_ZIP"
+
+ # Extract to temp directory first
+ EXTRACT_DIR="$TEMPDIR/extract"
+ mkdir -p "$EXTRACT_DIR"
+ unzip -o "$TEMPDIR/$PROTOC_ZIP" -d "$EXTRACT_DIR"
+
+ # Copy protoc binary
+ if [[ -f "$EXTRACT_DIR/bin/protoc" ]]; then
+ cp "$EXTRACT_DIR/bin/protoc" "$BINDIR/protoc"
+ elif [[ -f "$EXTRACT_DIR/bin/protoc.exe" ]]; then
+ cp "$EXTRACT_DIR/bin/protoc.exe" "$BINDIR/protoc"
+ else
+ echo "Error: protoc binary not found in archive"
+ exit 1
+ fi
+
+ # Copy include files
+ if [[ -d "$EXTRACT_DIR/include" ]]; then
+ cp -r "$EXTRACT_DIR/include"/* "$INCLUDE_DIR/"
+ fi
+
+ chmod +x "$BINDIR/protoc"
+ rm -rf "$EXTRACT_DIR"
+ rm -f "$TEMPDIR/$PROTOC_ZIP"
+fi
+
+# -----------------------------
+# Export PATH for local protoc
+# -----------------------------
+export PATH="$BINDIR:$GOPATH/bin:$PATH"
+rm -f "$GOPATH/bin/protoc-gen-go" "$GOPATH/bin/protoc-gen-go-grpc"
+# -----------------------------
+# Install Go plugins (fixed versions)
+# -----------------------------
+echo "Current go version:"
+go version
+if ! command -v protoc-gen-go &>/dev/null; then
+ echo "Installing protoc-gen-go v1.26..."
+ GO111MODULE=on GOPROXY=https://goproxy.cn,direct
GOSUMDB=sum.golang.google.cn go install
google.golang.org/protobuf/cmd/protoc-gen-go@v1.26.0
Review Comment:
Please remove these env, just do `go install`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
