Skip to content

Use native git variants by default with go-git variants as build tag #13673

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

Merged
merged 29 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c76d2ad
Move last commit cache back into modules/git
zeripath Nov 15, 2020
fd69b5b
Remove go-git from the interface for last commit cache
zeripath Nov 15, 2020
2013955
move cacheref to last_commit_cache
zeripath Nov 15, 2020
58f0579
Remove go-git from routers/private/hook
zeripath Nov 15, 2020
d45a18f
Move FindLFSFiles to pipeline
zeripath Nov 15, 2020
164a02e
Make no-go-git variants
zeripath Nov 12, 2020
764948f
Submodule RefID
zeripath Nov 22, 2020
d8d498d
fix issue with GetCommitsInfo
zeripath Nov 22, 2020
abddc4e
fix GetLastCommitForPaths
zeripath Nov 22, 2020
1e396b7
Improve efficiency
zeripath Nov 23, 2020
4b35e94
More efficiency
zeripath Nov 24, 2020
95d6652
even faster
zeripath Nov 28, 2020
20632e2
Reduce duplication
zeripath Nov 28, 2020
a35cf18
As per @lunny
zeripath Nov 28, 2020
b047f93
attempt to fix drone
zeripath Nov 30, 2020
e89fd10
Merge remote-tracking branch 'origin/master' into no-go-git
zeripath Dec 8, 2020
55f4e03
fix test-tags
zeripath Dec 8, 2020
bb593f4
Merge branch 'master' into no-go-git
zeripath Dec 8, 2020
8c887fc
default to use no-go-git variants and add gogit build tag
zeripath Dec 10, 2020
18da9de
Merge branch 'no-go-git' of github.com:zeripath/gitea into no-go-git
zeripath Dec 10, 2020
e1b923e
Merge remote-tracking branch 'origin/master' into no-go-git
zeripath Dec 10, 2020
1897dad
placate lint
zeripath Dec 10, 2020
531da0f
Merge branch 'master' into no-go-git
zeripath Dec 10, 2020
03c8cd6
Merge branch 'master' into no-go-git
zeripath Dec 10, 2020
4633679
Merge branch 'master' into no-go-git
6543 Dec 11, 2020
320adb4
as per @6543
zeripath Dec 12, 2020
5582fba
Merge branch 'master' into no-go-git
zeripath Dec 14, 2020
39c8dfe
Merge branch 'master' into no-go-git
techknowlogick Dec 16, 2020
29a5951
Merge branch 'master' into no-go-git
6543 Dec 17, 2020
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
29 changes: 26 additions & 3 deletions .drone.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ steps:
GOSUMDB: sum.golang.org
TAGS: bindata sqlite sqlite_unlock_notify

- name: lint-backend-gogit
pull: always
image: golang:1.15
commands:
- make lint-backend
environment:
GOPROXY: https://goproxy.cn # proxy.golang.org is blocked in China, this proxy is not
GOSUMDB: sum.golang.org
TAGS: bindata gogit sqlite sqlite_unlock_notify

Copy link
Member

Choose a reason for hiding this comment

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

Sorry I'm late here but is it really necessary to run the linter twice here? Do TAGS really affect the linting result? I thought TAGS only affect the build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

Copy link
Member

Choose a reason for hiding this comment

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

I guess it's beyond me to understand how TAGS can affect linting (which should just be a static analysis of files). Care to explain? Does the go linter actually do runtime analysis as well?

Copy link
Member

Choose a reason for hiding this comment

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

golangci-lint has a --build-tags to support lint different tags.

Copy link
Member

Choose a reason for hiding this comment

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

Ok I guess I need to study golangci-lint more. I was under the impression that linters in general just do static analysis of file contents, but I guess this linter does more than that.

Copy link
Member

@silverwind silverwind Dec 17, 2020

Choose a reason for hiding this comment

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

Thought I generally wonder if it's really necessary to do this double linting every single CI run. The go linting already takes around 3 minutes per run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is that the go-git variants require linting too.

Copy link
Member

@silverwind silverwind Dec 17, 2020

Choose a reason for hiding this comment

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

Can't we just pass a list of files to the linters as argument? I think it should be possible.

- name: checks-frontend
image: node:14
commands:
Expand Down Expand Up @@ -69,7 +79,7 @@ steps:
GOPROXY: off
GOOS: linux
GOARCH: arm64
TAGS: bindata
TAGS: bindata gogit
commands:
- make backend # test cross compile
- rm ./gitea # clean
Expand Down Expand Up @@ -173,6 +183,17 @@ steps:
GITHUB_READ_TOKEN:
from_secret: github_read_token

- name: unit-test-gogit
pull: always
image: golang:1.15
commands:
- make unit-test-coverage test-check
environment:
GOPROXY: off
TAGS: bindata gogit sqlite sqlite_unlock_notify
GITHUB_READ_TOKEN:
from_secret: github_read_token

- name: test-mysql
image: golang:1.15
commands:
Expand Down Expand Up @@ -305,7 +326,8 @@ steps:
- timeout -s ABRT 40m make test-sqlite-migration test-sqlite
environment:
GOPROXY: off
TAGS: bindata
TAGS: bindata gogit sqlite sqlite_unlock_notify
TEST_TAGS: gogit sqlite sqlite_unlock_notify
USE_REPO_TEST_DIR: 1
depends_on:
- build
Expand All @@ -318,7 +340,8 @@ steps:
- timeout -s ABRT 40m make test-pgsql-migration test-pgsql
environment:
GOPROXY: off
TAGS: bindata
TAGS: bindata gogit
TEST_TAGS: gogit
TEST_LDAP: 1
USE_REPO_TEST_DIR: 1
depends_on:
Expand Down
19 changes: 11 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@ TAGS ?=
TAGS_SPLIT := $(subst $(COMMA), ,$(TAGS))
TAGS_EVIDENCE := $(MAKE_EVIDENCE_DIR)/tags

TEST_TAGS ?= sqlite sqlite_unlock_notify

GO_DIRS := cmd integrations models modules routers build services vendor tools

GO_SOURCES := $(wildcard *.go)
GO_SOURCES += $(shell find $(GO_DIRS) -type f -name "*.go" -not -path modules/options/bindata.go -not -path modules/public/bindata.go -not -path modules/templates/bindata.go)

Expand Down Expand Up @@ -339,8 +342,8 @@ watch-backend: go-check

.PHONY: test
test:
@echo "Running go test..."
@$(GO) test $(GOTESTFLAGS) -mod=vendor -tags='sqlite sqlite_unlock_notify' $(GO_PACKAGES)
@echo "Running go test with -tags '$(TEST_TAGS)'..."
@$(GO) test $(GOTESTFLAGS) -mod=vendor -tags='$(TEST_TAGS)' $(GO_PACKAGES)

.PHONY: test-check
test-check:
Expand All @@ -356,17 +359,17 @@ test-check:

.PHONY: test\#%
test\#%:
@echo "Running go test..."
@$(GO) test -mod=vendor -tags='sqlite sqlite_unlock_notify' -run $(subst .,/,$*) $(GO_PACKAGES)
@echo "Running go test with -tags '$(TEST_TAGS)'..."
@$(GO) test -mod=vendor -tags='$(TEST_TAGS)' -run $(subst .,/,$*) $(GO_PACKAGES)

.PHONY: coverage
coverage:
GO111MODULE=on $(GO) run -mod=vendor build/gocovmerge.go integration.coverage.out $(shell find . -type f -name "coverage.out") > coverage.all

.PHONY: unit-test-coverage
unit-test-coverage:
@echo "Running unit-test-coverage..."
@$(GO) test $(GOTESTFLAGS) -mod=vendor -tags='sqlite sqlite_unlock_notify' -cover -coverprofile coverage.out $(GO_PACKAGES) && echo "\n==>\033[32m Ok\033[m\n" || exit 1
@echo "Running unit-test-coverage -tags '$(TEST_TAGS)'..."
@$(GO) test $(GOTESTFLAGS) -mod=vendor -tags='$(TEST_TAGS)' -cover -coverprofile coverage.out $(GO_PACKAGES) && echo "\n==>\033[32m Ok\033[m\n" || exit 1

.PHONY: vendor
vendor:
Expand Down Expand Up @@ -511,7 +514,7 @@ integrations.mssql.test: git-check $(GO_SOURCES)
$(GO) test $(GOTESTFLAGS) -mod=vendor -c code.gitea.io/gitea/integrations -o integrations.mssql.test

integrations.sqlite.test: git-check $(GO_SOURCES)
$(GO) test $(GOTESTFLAGS) -mod=vendor -c code.gitea.io/gitea/integrations -o integrations.sqlite.test -tags 'sqlite sqlite_unlock_notify'
$(GO) test $(GOTESTFLAGS) -mod=vendor -c code.gitea.io/gitea/integrations -o integrations.sqlite.test -tags '$(TEST_TAGS)'

integrations.cover.test: git-check $(GO_SOURCES)
$(GO) test $(GOTESTFLAGS) -mod=vendor -c code.gitea.io/gitea/integrations -coverpkg $(shell echo $(GO_PACKAGES) | tr ' ' ',') -o integrations.cover.test
Expand All @@ -534,7 +537,7 @@ migrations.mssql.test: $(GO_SOURCES)

.PHONY: migrations.sqlite.test
migrations.sqlite.test: $(GO_SOURCES)
$(GO) test $(GOTESTFLAGS) -c code.gitea.io/gitea/integrations/migration-test -o migrations.sqlite.test -tags 'sqlite sqlite_unlock_notify'
$(GO) test $(GOTESTFLAGS) -c code.gitea.io/gitea/integrations/migration-test -o migrations.sqlite.test -tags '$(TEST_TAGS)'

.PHONY: check
check: test
Expand Down
1 change: 1 addition & 0 deletions docs/content/doc/installation/from-source.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ Depending on requirements, the following build tags can be included.
- `pam`: Enable support for PAM (Linux Pluggable Authentication Modules). Can
be used to authenticate local users or extend authentication to methods
available to PAM.
* `gogit`: (EXPERIMENTAL) Use go-git variants of git commands.

Bundling assets into the binary using the `bindata` build tag is recommended for
production deployments. It is possible to serve the static assets directly via a reverse proxy,
Expand Down
23 changes: 23 additions & 0 deletions modules/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,24 @@ func newCache(cacheConfig setting.Cache) (mc.Cache, error) {
})
}

// Cache is the interface that operates the cache data.
type Cache interface {
// Put puts value into cache with key and expire time.
Put(key string, val interface{}, timeout int64) error
// Get gets cached value by given key.
Get(key string) interface{}
// Delete deletes cached value by given key.
Delete(key string) error
// Incr increases cached int-type value by given key as a counter.
Incr(key string) error
// Decr decreases cached int-type value by given key as a counter.
Decr(key string) error
// IsExist returns true if cached value exists.
IsExist(key string) bool
// Flush deletes all cached data.
Flush() error
}

// NewContext start cache service
func NewContext() error {
var err error
Expand All @@ -40,6 +58,11 @@ func NewContext() error {
return err
}

// GetCache returns the currently configured cache
func GetCache() Cache {
return conn
}

// GetString returns the key value from cache with callback when no key exists in cache
func GetString(key string, getFunc func() (string, error)) (string, error) {
if conn == nil || setting.CacheService.TTL == 0 {
Expand Down
70 changes: 0 additions & 70 deletions modules/cache/last_commit.go

This file was deleted.

3 changes: 1 addition & 2 deletions modules/convert/git_commit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@ import (
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/util"

"github.com/go-git/go-git/v5/plumbing/object"
"github.com/stretchr/testify/assert"
)

func TestToCommitMeta(t *testing.T) {
assert.NoError(t, models.PrepareTestDatabase())
headRepo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository)
sha1, _ := git.NewIDFromString("0000000000000000000000000000000000000000")
signature := &object.Signature{Name: "Test Signature", Email: "[email protected]", When: time.Unix(0, 0)}
signature := &git.Signature{Name: "Test Signature", Email: "[email protected]", When: time.Unix(0, 0)}
tag := &git.Tag{
Name: "Test Tag",
ID: sha1,
Expand Down
Loading