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

Run golangci-lint with GOOS=linux/darwin/windows #1260

Closed
wants to merge 16 commits into from
Closed

Run golangci-lint with GOOS=linux/darwin/windows #1260

wants to merge 16 commits into from

Conversation

xzfc
Copy link
Contributor

@xzfc xzfc commented Apr 6, 2022

Description

Currently, our CI doesn't check non-linux platform-specific code. Checking these files may be useful for those who use non-Linux OS as their dev environment.

This PR enables checks for Windows-only and macOS-only files by running GOOS=windows golangci-lint and GOOS=darwin golangci-lint on Ubuntu.

An alternative option is to run this action on windows-latest and macos-lates natively as proposed in the doc, but it fails on Windows due to the path separators difference in exclude-rules in .golangci.yml (see golangci/golangci-lint#1398).

So, this PR solves the issue completely for darwin (i.e. you could run golangci-lint on macOS without errors), and partially for Windows (i.e. Windows-specific files like grpc_utils_windows.go are linted, but golangci-lint is still not usable on Windows).


Also, this PR removes an unused function GetInode(). The reason is that the linter complains about the Windows implementation of this function.

pkg/tools/fs/inode_windows_amd64.go:38:17: G103: Use of unsafe calls should be audited (gosec)
	ptr := uintptr(unsafe.Pointer(stat))

As I can see in the code, it casts a pointer to some internal data structure into an integer and treats it as an inode number. I don't think this would produce any meaningful result. And I am not sure how to properly fix this function on Windows since there are no inode numbers in Windows. But since this function isn't used anywhere except an archived repo cmd-nsc-proxy, I've deleted it.

Issue link

How Has This Been Tested?

  • Added unit testing to cover
  • Tested manually
  • Tested by integration testing
  • Have not tested

Types of changes

  • Bug fix
  • New functionallity
  • Documentation
  • Refactoring
  • CI

edwarnicke and others added 15 commits March 14, 2022 04:33
Also improved greatly documentation in various places

Should resolve #1234

Signed-off-by: Ed Warnicke <[email protected]>
…1236)

Bumps [github.com/nats-io/nats-streaming-server](https://github.com/nats-io/nats-streaming-server) from 0.24.1 to 0.24.3.
- [Release notes](https://github.com/nats-io/nats-streaming-server/releases)
- [Changelog](https://github.com/nats-io/nats-streaming-server/blob/main/.goreleaser.yml)
- [Commits](nats-io/nats-streaming-server@v0.24.1...v0.24.3)

---
updated-dependencies:
- dependency-name: github.com/nats-io/nats-streaming-server
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* add forwarder matching

Signed-off-by: Nikita Skrynnik <[email protected]>

* use path index instead of magic number

Signed-off-by: Nikita Skrynnik <[email protected]>

* minor fixes for tests

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix linter issue

Signed-off-by: Nikita Skrynnik <[email protected]>

* make samples

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix linter issues

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix heal tests

Signed-off-by: Nikita Skrynnik <[email protected]>

* change iter count

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix timeout value

Signed-off-by: Nikita Skrynnik <[email protected]>

* change iter count again

Signed-off-by: Nikita Skrynnik <[email protected]>

* change count

Signed-off-by: Nikita Skrynnik <[email protected]>

* increase timeout

Signed-off-by: Nikita Skrynnik <[email protected]>

* apply suggestions

Signed-off-by: Nikita Skrynnik <[email protected]>

* add debug info

Signed-off-by: Nikita Skrynnik <[email protected]>

* forwarder health check after nsmgr restart

Signed-off-by: Nikita Skrynnik <[email protected]>

* disable traces

Signed-off-by: Nikita Skrynnik <[email protected]>

* cleanup

Signed-off-by: Nikita Skrynnik <[email protected]>

* rework health check

Signed-off-by: Nikita Skrynnik <[email protected]>

* enable tracing

Signed-off-by: Nikita Skrynnik <[email protected]>

* disable tracing

Signed-off-by: Nikita Skrynnik <[email protected]>

* increase timeout

Signed-off-by: Nikita Skrynnik <[email protected]>

* rework forwarder health check again

Signed-off-by: Nikita Skrynnik <[email protected]>

* add simple sleep

Signed-off-by: Nikita Skrynnik <[email protected]>

* disable tracing

Signed-off-by: Nikita Skrynnik <[email protected]>

* increase sleep time

Signed-off-by: Nikita Skrynnik <[email protected]>

* add healing

Signed-off-by: Nikita Skrynnik <[email protected]>

* tracing

Signed-off-by: Nikita Skrynnik <[email protected]>

* add delete

Signed-off-by: Nikita Skrynnik <[email protected]>

* disable race

Signed-off-by: Nikita Skrynnik <[email protected]>

* revert 'add delete' commit

Signed-off-by: Nikita Skrynnik <[email protected]>

* enable tracing

Signed-off-by: Nikita Skrynnik <[email protected]>

* add client conn delete

Signed-off-by: Nikita Skrynnik <[email protected]>

* delete registry restart

Signed-off-by: Nikita Skrynnik <[email protected]>

* test with forwarder health check

Signed-off-by: Nikita Skrynnik <[email protected]>

* remove health check + increase sleep

Signed-off-by: Nikita Skrynnik <[email protected]>

* change iter count

Signed-off-by: Nikita Skrynnik <[email protected]>

* add forwarder registration

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix linter issue

Signed-off-by: Nikita Skrynnik <[email protected]>

* enable all tests

Signed-off-by: Nikita Skrynnik <[email protected]>

* test with health check again

Signed-off-by: Nikita Skrynnik <[email protected]>

* run only one test

Signed-off-by: Nikita Skrynnik <[email protected]>

* add forwarder register

Signed-off-by: Nikita Skrynnik <[email protected]>

* test with clientconn delete again

Signed-off-by: Nikita Skrynnik <[email protected]>

* revert last commit

Signed-off-by: Nikita Skrynnik <[email protected]>

* increase timeout

Signed-off-by: Nikita Skrynnik <[email protected]>

* add log

Signed-off-by: Nikita Skrynnik <[email protected]>

* test with forwarder register again

Signed-off-by: Nikita Skrynnik <[email protected]>

* increase forwarder count

Signed-off-by: Nikita Skrynnik <[email protected]>

* add race flag + disable tracing

Signed-off-by: Nikita Skrynnik <[email protected]>

* move test to heal_test.go file

Signed-off-by: Nikita Skrynnik <[email protected]>

* add clientconn.Delete(ctx) again

Signed-off-by: Nikita Skrynnik <[email protected]>

* test only local case

Signed-off-by: Nikita Skrynnik <[email protected]>

* test with registry restart

Signed-off-by: Nikita Skrynnik <[email protected]>

* cleanup

Signed-off-by: Nikita Skrynnik <[email protected]>

* test retry patch

Signed-off-by: Nikita Skrynnik <[email protected]>

* add logs

Signed-off-by: Nikita Skrynnik <[email protected]>

* wake CI

Signed-off-by: Nikita Skrynnik <[email protected]>

* test with 100 ms

Signed-off-by: Nikita Skrynnik <[email protected]>

* test with 1s

Signed-off-by: Nikita Skrynnik <[email protected]>

* test without retry

Signed-off-by: Nikita Skrynnik <[email protected]>

* add manual forwarder heal

Signed-off-by: Nikita Skrynnik <[email protected]>

* add remote test

Signed-off-by: Nikita Skrynnik <[email protected]>

* reduce tryTimeout

Signed-off-by: Nikita Skrynnik <[email protected]>

* add nse reregistration

Signed-off-by: Nikita Skrynnik <[email protected]>

* cleanup

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix linter issue

Signed-off-by: Nikita Skrynnik <[email protected]>

* enable all tests

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix linter issue

Signed-off-by: Nikita Skrynnik <[email protected]>

* apply suggestions

Signed-off-by: Nikita Skrynnik <[email protected]>
* add unit test for mutually aware nses use-case

Signed-off-by: Nikita Skrynnik <[email protected]>

* add getFullURL func

Signed-off-by: Nikita Skrynnik <[email protected]>

* add awarenessGroups option for client

Signed-off-by: Nikita Skrynnik <[email protected]>

* implement awarenessGroups check in excludeprefixesClient

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix tests

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix linter issues

Signed-off-by: Nikita Skrynnik <[email protected]>

* improve unit test

Signed-off-by: Nikita Skrynnik <[email protected]>

* modify Close method

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix linter issue

Signed-off-by: Nikita Skrynnik <[email protected]>

* rework unit test + fix match_selector

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix tests

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix linter issues

Signed-off-by: Nikita Skrynnik <[email protected]>

* apply comments

Signed-off-by: Nikita Skrynnik <[email protected]>

* add more usecases for awarenessgroups decoder

Signed-off-by: Nikita Skrynnik <[email protected]>

* refactor awarenessGroups

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix linter issues

Signed-off-by: Nikita Skrynnik <[email protected]>

* use atomic.LoadInt32 func

Signed-off-by: Nikita Skrynnik <[email protected]>
* delete label matching (for ci)

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix Test_AwareNSEs

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix Test_AwareNSEs timeout

Signed-off-by: Nikita Skrynnik <[email protected]>
* Add cleanup chain element

Signed-off-by: Artem Glazychev <[email protected]>

* Rename option

Signed-off-by: Artem Glazychev <[email protected]>
* add vl3 chain elements and vl3ipam

Signed-off-by: Denis Tingaikin <[email protected]>

* fix typos

Signed-off-by: Denis Tingaikin <[email protected]>
* add cmd-ipam-vl3 for update

Signed-off-by: Denis Tingaikin <[email protected]>

* subscribe cmd-map-ip-k8s on updates from sdk

Signed-off-by: Denis Tingaikin <[email protected]>
* try to reproduce bug

Signed-off-by: Nikita Skrynnik <[email protected]>

* test only local case

Signed-off-by: Nikita Skrynnik <[email protected]>

* add logs

Signed-off-by: Nikita Skrynnik <[email protected]>

* change order of forwarders

Signed-off-by: Nikita Skrynnik <[email protected]>

* add more test launches

Signed-off-by: Nikita Skrynnik <[email protected]>

* run test 100 times

Signed-off-by: Nikita Skrynnik <[email protected]>

* run test 50 times

Signed-off-by: Nikita Skrynnik <[email protected]>

* enable trace logs again

Signed-off-by: Nikita Skrynnik <[email protected]>

* disable healing

Signed-off-by: Nikita Skrynnik <[email protected]>

* launch test 80 times

Signed-off-by: Nikita Skrynnik <[email protected]>

* launch 90 times

Signed-off-by: Nikita Skrynnik <[email protected]>

* launch 99 times

Signed-off-by: Nikita Skrynnik <[email protected]>

* rework node client

Signed-off-by: Nikita Skrynnik <[email protected]>

* fix cii

Signed-off-by: Nikita Skrynnik <[email protected]>

* apply comments

Signed-off-by: Nikita Skrynnik <[email protected]>
* don't spawn logs if context has cancelled gracefully

Signed-off-by: denis-tingaikin <[email protected]>

* fix linter

Signed-off-by: denis-tingaikin <[email protected]>
@denis-tingaikin
Copy link
Member

denis-tingaikin commented Apr 6, 2022

@xzfc Should we consider non-linux linter running on CI?

@xzfc
Copy link
Contributor Author

xzfc commented Apr 7, 2022

I'd think it would be an overkill.

An option to lint all files regardless of the platform was proposed in golangci/golangci-lint#1646, but not implemented.
Other options are to run golangci-lint multiple times with different flags (e.g. golangci-lint --build-tags linux && golangci-lint --build-tags windows) or to introduce an additional all tag to all platform-specific code (e.g. // +build linux all).

@denis-tingaikin
Copy link
Member

(e.g. golangci-lint --build-tags linux && golangci-lint --build-tags windows) or to introduce an additional all tag to all platform-specific code (e.g. // +build linux all).

Could you please add a step for golangci job with non linux tag?

* gofmt *_nolinux.go
* Remove unused function GetInode()

Signed-off-by: Albert Safin <[email protected]>
@xzfc
Copy link
Contributor Author

xzfc commented Apr 8, 2022

Could you please add a step for golangci job with non linux tag?

Done, see the updated PR description.

@xzfc xzfc changed the title Gofmt on *_nolinux.go Run golangci-lint with GOOS=linux/darwin/windows Apr 8, 2022
@xzfc xzfc closed this by deleting the head repository May 23, 2023
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

Successfully merging this pull request may close these issues.

7 participants