Skip to content

CI: spring cleaning Makefile #3916

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.

Sign up for GitHub

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

Merged
AkihiroSuda merged 1 commit into from
Feb 27, 2025
Merged

CI: spring cleaning Makefile #3916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/lint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,13 @@ jobs:
with:
go-version: ${{ env.GO_VERSION }}
check-latest: true
- name: install required linters and dev-tools
run: |
make install-dev-tools
- name: yaml
run: make lint-yaml
- name: shell
run: make lint-shell
- name: go imports ordering
run: |
go install -v github.com/incu6us/goimports-reviser/v3@latest
make lint-imports
5 changes: 5 additions & 0 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,12 @@ linters-settings:
- unnecessaryBlock

issues:
max-issues-per-linter: 0
Copy link
Member

@AkihiroSuda AkihiroSuda Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this ?

Copy link
Contributor Author

@apostasie apostasie Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that golangci does not strip the output and show all the errors instead.
It is frustrating to run golangci, fix the issues, then have to run it again for "more" issues.

wdyt?

max-same-issues: 0
exclude-rules:
- linters:
- revive
text: "unused-parameter"

output:
sort-results: true
232 changes: 196 additions & 36 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,85 +18,240 @@
# Licensed under the Apache License, Version 2.0
# -----------------------------------------------------------------------------

##########################
# Configuration
##########################
PACKAGE := "github.com/containerd/nerdctl/v2"
ORG_PREFIXES := "github.com/containerd"

DOCKER ?= docker
GO ?= go
GOOS ?= $(shell $(GO) env GOOS)
ifeq ($(GOOS),windows)
BIN_EXT := .exe
endif

PACKAGE := github.com/containerd/nerdctl/v2

# distro builders might wanna override these
# distro builders might want to override these
PREFIX ?= /usr/local
BINDIR ?= $(PREFIX)/bin
DATADIR ?= $(PREFIX)/share
DOCDIR ?= $(DATADIR)/doc

BINARY ?= "nerdctl"
MAKEFILE_DIR := $(patsubst %/,%,$(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
VERSION ?= $(shell git -C $(MAKEFILE_DIR) describe --match 'v[0-9]*' --dirty='.m' --always --tags)
VERSION_TRIMMED := $(VERSION:v%=%)
REVISION ?= $(shell git -C $(MAKEFILE_DIR) rev-parse HEAD)$(shell if ! git -C $(MAKEFILE_DIR) diff --no-ext-diff --quiet --exit-code; then echo .m; fi)
LINT_COMMIT_RANGE ?= main..HEAD
Copy link
Member

@AkihiroSuda AkihiroSuda Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to work for release/X.X branches, which are created when we are not ready to make a release from the main branch

Copy link
Contributor Author

@apostasie apostasie Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm. Let me test that immediately.

Copy link
Contributor Author

@apostasie apostasie Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you do not want the range compared to main:

git checkout origin/release/1.7
touch foo; git add foo; git commit -S -s -m "Foo"
LINT_COMMIT_RANGE=origin/release/1.7..HEAD make lint-commits

INFO[0000] using commit range: origin/release/1.7..HEAD
 * ff0ac028 "Foo" ... PASS

Can you let me know what use-case you would like?

Copy link
Member

@AkihiroSuda AkihiroSuda Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overriding LINT_COMMIT_RANGE seems good.
This should be set in the GHA yaml with $GITHUB_BASE_REF

Copy link
Contributor Author

@apostasie apostasie Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Let me look at the workflow files right now.

Copy link
Contributor Author

@apostasie apostasie Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refreshing myself on this: we do not run on the CI make lint-commits (or the overarching make lint).

The reason is that project-checks does do the git-validation for us already (and that would just duplicate the check).

I am going to add a note to the md file about proper local use of make lint-commits though.

Copy link
Contributor Author

@apostasie apostasie Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Added a few words in the testing markdown doc about this specific point.

Let me know your thoughts.

GO_BUILD_LDFLAGS ?= -s -w
GO_BUILD_FLAGS ?=

##########################
# Helpers
##########################
ifdef VERBOSE
VERBOSE_FLAG := -v
VERBOSE_FLAG_LONG := --verbose
endif

GO_BUILD_LDFLAGS ?= -s -w
GO_BUILD_FLAGS ?=
export GO_BUILD=CGO_ENABLED=0 GOOS=$(GOOS) $(GO) -C $(MAKEFILE_DIR) build -ldflags "$(GO_BUILD_LDFLAGS) $(VERBOSE_FLAG) -X $(PACKAGE)/pkg/version.Version=$(VERSION) -X $(PACKAGE)/pkg/version.Revision=$(REVISION)"

ifndef NO_COLORS
NC := \033[0m
GREEN := \033[1;32m
ORANGE := \033[1;33m
endif

recursive_wildcard=$(wildcard $1$2) $(foreach e,$(wildcard $1*),$(call recursive_wildcard,$e/,$2))

define title
@printf "$(GREEN)____________________________________________________________________________________________________\n"
@printf "$(GREEN)%*s\n" $$(( ( $(shell echo "🤓$(1) 🤓" | wc -c ) + 100 ) / 2 )) "🤓$(1) 🤓"
@printf "$(GREEN)____________________________________________________________________________________________________\n$(ORANGE)"
endef

define footer
@printf "$(GREEN)> %s: done!\n" "$(1)"
@printf "$(GREEN)____________________________________________________________________________________________________\n$(NC)"
endef

##########################
# High-level tasks definitions
##########################
all: binaries

lint: lint-go-all lint-imports lint-yaml lint-shell lint-commits lint-mod lint-licenses-all

fix: fix-mod fix-imports fix-go-all

# TODO: fix race task and add it
test: test-unit # test-unit-race test-unit-bench

help:
@echo "Usage: make <target>"
@echo
@echo " * 'install' - Install binaries to system locations."
@echo " * 'lint' - Run linters against codebase."
@echo " * 'fix' - Automatically fixes imports, modules, and simple formatting."
@echo " * 'test' - Run basic unit testing."
@echo " * 'binaries' - Build nerdctl."
@echo " * 'install' - Install binaries to system locations."
@echo " * 'clean' - Clean artifacts."
@echo " * 'lint' - Run various linters."

nerdctl:
$(GO_BUILD) $(GO_BUILD_FLAGS) $(VERBOSE_FLAG) -o $(CURDIR)/_output/nerdctl$(BIN_EXT) ./cmd/nerdctl
##########################
# Building and installation tasks
##########################
binaries: $(CURDIR)/_output/$(BINARY)$(BIN_EXT)

$(CURDIR)/_output/$(BINARY)$(BIN_EXT):
$(call title, $@)
$(GO_BUILD) $(GO_BUILD_FLAGS) $(VERBOSE_FLAG) -o $(CURDIR)/_output/$(BINARY)$(BIN_EXT) ./cmd/nerdctl
$(call footer, $@)

install:
$(call title, $@)
install -D -m 755 $(CURDIR)/_output/$(BINARY) $(DESTDIR)$(BINDIR)/$(BINARY)
install -D -m 755 $(MAKEFILE_DIR)/extras/rootless/containerd-rootless.sh $(DESTDIR)$(BINDIR)/containerd-rootless.sh
install -D -m 755 $(MAKEFILE_DIR)/extras/rootless/containerd-rootless-setuptool.sh $(DESTDIR)$(BINDIR)/containerd-rootless-setuptool.sh
install -D -m 644 -t $(DESTDIR)$(DOCDIR)/nerdctl $(MAKEFILE_DIR)/docs/*.md
$(call footer, $@)

clean:
$(call title, $@)
find . -name \*~ -delete
find . -name \#\* -delete
rm -rf $(CURDIR)/_output/* $(MAKEFILE_DIR)/vendor
$(call footer, $@)

lint: lint-go lint-imports lint-yaml lint-shell

##########################
# Linting tasks
##########################
lint-go:
cd $(MAKEFILE_DIR) && GOOS=linux golangci-lint run $(VERBOSE_FLAG_LONG) ./... && \
GOOS=windows golangci-lint run $(VERBOSE_FLAG_LONG) ./... && \
GOOS=freebsd golangci-lint run $(VERBOSE_FLAG_LONG) ./...
$(call title, $@: $(GOOS))
@cd $(MAKEFILE_DIR) \
&& golangci-lint run $(VERBOSE_FLAG_LONG) ./...
$(call footer, $@)

lint-go-all:
$(call title, $@)
@cd $(MAKEFILE_DIR) \
&& GOOS=linux make lint-go \
&& GOOS=windows make lint-go \
&& GOOS=freebsd make lint-go
$(call footer, $@)

lint-imports:
cd $(MAKEFILE_DIR) && goimports-reviser -recursive -list-diff -set-exit-status -output stdout -company-prefixes "github.com/containerd" ./...

lint-fix-imports:
cd $(MAKEFILE_DIR) && goimports-reviser -company-prefixes "github.com/containerd" ./...
$(call title, $@)
@cd $(MAKEFILE_DIR) \
&& goimports-reviser -recursive -list-diff -set-exit-status -output stdout -company-prefixes "$(ORG_PREFIXES)" ./...
$(call footer, $@)

lint-yaml:
cd $(MAKEFILE_DIR) && yamllint .
$(call title, $@)
cd $(MAKEFILE_DIR) \
&& yamllint .
$(call footer, $@)

lint-shell: $(call recursive_wildcard,$(MAKEFILE_DIR)/,*.sh)
$(call title, $@)
shellcheck -a -x $^

$(call footer, $@)

lint-commits:
$(call title, $@)
@cd $(MAKEFILE_DIR) \
&& git-validation $(VERBOSE_FLAG) -run DCO,short-subject,dangling-whitespace -range "$(LINT_COMMIT_RANGE)"
$(call footer, $@)

lint-mod:
$(call title, $@)
@cd $(MAKEFILE_DIR) \
&& go mod tidy --diff
$(call footer, $@)

lint-licenses:
$(call title, $@: $(GOOS))
@cd $(MAKEFILE_DIR) \
&& ./hack/make-lint-licenses.sh
$(call footer, $@)

lint-licenses-all:
$(call title, $@)
@cd $(MAKEFILE_DIR) \
&& GOOS=linux make lint-licenses \
&& GOOS=freebsd make lint-licenses \
&& GOOS=windows make lint-licenses
$(call footer, $@)

##########################
# Automated fixing tasks
##########################
fix-go:
$(call title, $@: $(GOOS))
@cd $(MAKEFILE_DIR) \
&& golangci-lint run --fix
$(call footer, $@)

fix-go-all:
$(call title, $@)
@cd $(MAKEFILE_DIR) \
&& GOOS=linux make fix-go \
&& GOOS=freebsd make fix-go \
&& GOOS=windows make fix-go
$(call footer, $@)

fix-imports:
$(call title, $@)
@cd $(MAKEFILE_DIR) \
&& goimports-reviser -company-prefixes $(ORG_PREFIXES) ./...
$(call footer, $@)

fix-mod:
$(call title, $@)
@cd $(MAKEFILE_DIR) \
&& go mod tidy
$(call footer, $@)

##########################
# Development tools installation
##########################
install-dev-tools:
$(call title, $@)
# golangci: v1.64.5
# git-validation: main from 2023/11
# ltag: v0.2.5
# go-licenses: v2.0.0-alpha.1
# goimports-reviser: v3.8.2
@cd $(MAKEFILE_DIR) \
&& go install github.com/golangci/golangci-lint/cmd/golangci-lint@0a603e49e5e9870f5f9f2035bcbe42cd9620a9d5 \
&& go install github.com/vbatts/git-validation@679e5cad8c50f1605ab3d8a0a947aaf72fb24c07 \
&& go install github.com/kunalkushwaha/ltag@b0cfa33e4cc9383095dc584d3990b62c95096de0 \
&& go install github.com/google/go-licenses/v2@d01822334fba5896920a060f762ea7ecdbd086e8 \
&& go install github.com/incu6us/goimports-reviser/v3@f034195cc8a7ffc7cc70d60aa3a25500874eaf04 \
&& go install gotest.tools/gotestsum@ac6dad9c7d87b969004f7749d1942938526c9716
@echo "Remember to add GOROOT/bin to your path"
$(call footer, $@)

##########################
# Testing tasks
##########################
test-unit:
go test -v $(MAKEFILE_DIR)/pkg/...

binaries: nerdctl

install:
install -D -m 755 $(CURDIR)/_output/nerdctl $(DESTDIR)$(BINDIR)/nerdctl
install -D -m 755 $(MAKEFILE_DIR)/extras/rootless/containerd-rootless.sh $(DESTDIR)$(BINDIR)/containerd-rootless.sh
install -D -m 755 $(MAKEFILE_DIR)/extras/rootless/containerd-rootless-setuptool.sh $(DESTDIR)$(BINDIR)/containerd-rootless-setuptool.sh
install -D -m 644 -t $(DESTDIR)$(DOCDIR)/nerdctl $(MAKEFILE_DIR)/docs/*.md

$(call title, $@)
@go test $(VERBOSE_FLAG) $(MAKEFILE_DIR)/pkg/...
$(call footer, $@)

test-unit-bench:
$(call title, $@)
@go test $(VERBOSE_FLAG) $(MAKEFILE_DIR)/pkg/... -bench=.
$(call footer, $@)

test-unit-race:
$(call title, $@)
@go test $(VERBOSE_FLAG) $(MAKEFILE_DIR)/pkg/... -race
$(call footer, $@)

##########################
# Release tasks
##########################
# Note that these options will not work on macOS - unless you use gnu-tar instead of tar
TAR_OWNER0_FLAGS=--owner=0 --group=0
TAR_FLATTEN_FLAGS=--transform 's/.*\///g'
Expand All @@ -107,6 +262,7 @@ define make_artifact_full_linux
endef

artifacts: clean
$(call title, $@)
GOOS=linux GOARCH=amd64 make -C $(CURDIR) -f $(MAKEFILE_DIR)/Makefile binaries
tar $(TAR_OWNER0_FLAGS) $(TAR_FLATTEN_FLAGS) -czvf $(CURDIR)/_output/nerdctl-$(VERSION_TRIMMED)-linux-amd64.tar.gz $(CURDIR)/_output/nerdctl $(MAKEFILE_DIR)/extras/rootless/*

Expand Down Expand Up @@ -138,15 +294,19 @@ artifacts: clean

$(GO) -C $(MAKEFILE_DIR) mod vendor
tar $(TAR_OWNER0_FLAGS) -czf $(CURDIR)/_output/nerdctl-$(VERSION_TRIMMED)-go-mod-vendor.tar.gz $(MAKEFILE_DIR)/go.mod $(MAKEFILE_DIR)/go.sum $(MAKEFILE_DIR)/vendor
$(call footer, $@)

.PHONY: \
all \
lint \
fix \
test \
help \
nerdctl \
clean \
binaries \
install \
clean \
lint-go lint-go-all lint-imports lint-yaml lint-shell lint-commits lint-mod lint-licenses lint-licenses-all \
fix-go fix-go-all fix-imports fix-mod \
install-dev-tools \
test-unit test-unit-race test-unit-bench \
artifacts
lint \
lint-yaml \
lint-go \
lint-shell
32 changes: 27 additions & 5 deletions docs/testing/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,40 @@ and principles about writing tests.

For more comprehensive information about nerdctl test tools, see [tools.md](tools.md).

## Lint
## Code, fix, lint, rinse, repeat

```
go mod tidy
golangci-lint run ./...
# Do hack

# When done:
# This will ensure proper import formatting, modules, and basic formatting of your code
make fix

# And this will run all linters and report if any modification is required
make lint
```

This works on macOS as well - just pass along `GOOS=linux`.
Note that both these tasks will work on any OS, including macOS.

Also note that `make lint` does call on subtask `make lint-commits` which is going to lint commits for the range
`main..HEAD` by default (this is fine for most development flows that expect to be merged in main).

If you are targeting a specific branch that is not `main` (for example working against `release/1.7`),
you likely want to explicitly set the commit range to that instead.

eg:
`LINT_COMMIT_RANGE=target_branch..HEAD make lint-commits`
or
`LINT_COMMIT_RANGE=target_branch..HEAD make lint`

## Unit testing

Run `go test -v ./pkg/...`
```
# This will run basic unit testing
make test
```

This task must be run on a supported OS (linux, windows, or freebsd).

## Integration testing

Expand Down
Loading