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

Add support for riscv64, the install-golint.sh can't work #9202

Closed
Xunop opened this issue Dec 4, 2023 · 3 comments · Fixed by #9210
Closed

Add support for riscv64, the install-golint.sh can't work #9202

Xunop opened this issue Dec 4, 2023 · 3 comments · Fixed by #9210
Assignees
Labels
priority/p3 agreed that this would be good to have, but no one is available at the moment.

Comments

@Xunop
Copy link
Contributor

Xunop commented Dec 4, 2023

Expected behavior

All unit tests should pass.

Actual behavior

RUN hack/golangci-lint.sh
Installing GolangCI-Lint
golangci/golangci-lint crit uname_arch_check 'riscv64' got converted to 'riscv64' which is not a GOARCH value.  Please file bug report at https://github.com/client9/shlib
FAILED hack/golangci-lint.sh
make: *** [Makefile:116: test] Error 1

Information

  • Skaffold version: v2.9.0
  • Operating system: linux/riscv64
  • Installed via: make install
  • Contents of skaffold.yaml: N/A

Steps to reproduce the behavior

  1. a clonable repository with the sample skaffold project
  2. make test

More Info

Upon examining the source code, I made the following changes for hack/install-golint.sh, so that install-golint.sh is compatible with the riscv64:

--- hack/install-golint.sh
+++ hack/install-golint.sh
@@ -71,6 +71,7 @@ is_supported_platform() {
     linux/amd64) found=0 ;;
     linux/386) found=0 ;;
     linux/arm64) found=0 ;;
+    linux/riscv64) found=0 ;;
   esac
   return $found
 }
@@ -189,6 +190,7 @@ uname_arch() {
     armv5*) arch="armv5" ;;
     armv6*) arch="armv6" ;;
     armv7*) arch="armv7" ;;
+    riscv64) arch="riscv64" ;;
   esac
   echo ${arch}
 }
@@ -227,6 +229,7 @@ uname_arch_check() {
     mips64le) return 0 ;;
     s390x) return 0 ;;
     amd64p32) return 0 ;;
+    riscv64) return 0 ;;
   esac
   log_crit "uname_arch_check '$(uname -m)' got converted to '$arch' which is not a GOARCH value.  Please file bug report at https://github.com/client9/shlib"
   return 1

Upon further investigation of the golangci-lint repository, I discovered that the included install.sh script(https://github.com/golangci/golangci-lint/blob/master/install.sh) supports a broader range of architectures. It may be advantageous to utilize this script instead. If this option is not feasible, I am prepared to create a pull request to introduce riscv64 support to the existing script.

@Xunop
Copy link
Contributor Author

Xunop commented Dec 4, 2023

When executing ./hack/bin/golangci-lint run --skip-dirs fs/assets/credits_generated --exclude=SA1019 --exclude=appendAssign -c hack/golangci.yml on a low-performance device (riscv64), it fails due to a timeout. To address this issue, I have adjusted the timeout option in the hack/golangci.yml file from 10 minutes to 20 minutes. Subsequent testing revealed that the process completed in 17 minutes:

PASSED hack/golangci-lint.sh in 1068s

I don't think this should be a problem, just a side note here.

@ericzzzzzzz ericzzzzzzz added the priority/p3 agreed that this would be good to have, but no one is available at the moment. label Dec 5, 2023
@ericzzzzzzz
Copy link
Contributor

Hi @Xunop , Thank you for creating the issue, feel free to create a pr to make the change, we can help with the review. Thanks

@Xunop
Copy link
Contributor Author

Xunop commented Dec 6, 2023

Hi @ericzzzzzzz , I have created a pull request for this issue:#9210

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/p3 agreed that this would be good to have, but no one is available at the moment.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants