From ca69ded83ebefbf898a24e4a26c0641cca8d58ad Mon Sep 17 00:00:00 2001 From: guillep2k <18600385+guillep2k@users.noreply.github.com> Date: Sat, 9 Nov 2019 17:15:37 -0300 Subject: [PATCH 1/8] Update Github migration test (#8896) Earlier today #716 was reopened which updated the modification time for an old milestone (1.6.0) that we use in testing with the assumption that it is old and won't change. This breaks all builds now, so remove this test since we have others that test the same code and this milestone will likely be updated again as that issue changes etc... --- modules/migrations/github_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/modules/migrations/github_test.go b/modules/migrations/github_test.go index 2a0a4edf3268..d9976d11326a 100644 --- a/modules/migrations/github_test.go +++ b/modules/migrations/github_test.go @@ -118,12 +118,6 @@ func TestGitHubDownloadRepo(t *testing.T) { "2018-09-05 16:34:22 +0000 UTC", "2018-08-11 08:45:01 +0000 UTC", "closed", milestone) - case "1.6.0": - assertMilestoneEqual(t, "1.6.0", "2018-09-25 07:00:00 +0000 UTC", - "2018-05-11 05:37:01 +0000 UTC", - "2019-01-27 19:21:22 +0000 UTC", - "2018-11-23 13:23:16 +0000 UTC", - "closed", milestone) case "1.7.0": assertMilestoneEqual(t, "1.7.0", "2018-12-25 08:00:00 +0000 UTC", "2018-08-28 14:20:14 +0000 UTC", From b27cac021f6f51c7605b29713f3293cdbcdc85f7 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 10 Nov 2019 00:06:38 +0000 Subject: [PATCH 2/8] Fix issue with user.fullname (#8903) --- templates/base/footer.tmpl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/templates/base/footer.tmpl b/templates/base/footer.tmpl index 13718620da0e..2a4b895dc2b3 100644 --- a/templates/base/footer.tmpl +++ b/templates/base/footer.tmpl @@ -63,11 +63,13 @@ noMatchTemplate: function () { return null }, menuItemTemplate: function (item) { var user = item.original; - var itemStr = '' + user.name + ''; + var item = $('
') + item.append($('', {'src': user.avatar})) + item.append($('', {'class': 'name'}).text(user.name)) if (user.fullname && user.fullname != '') { - itemStr += '' + user.fullname + ''; + item.append($('', {'class': 'fullname'}).text(user.fullname)) } - return itemStr; + return item.html(); } }); var content = document.getElementById('content'); From 8693e544269e45e8415cb5c9ce350ed07a1e3bf6 Mon Sep 17 00:00:00 2001 From: guillep2k <18600385+guillep2k@users.noreply.github.com> Date: Sat, 9 Nov 2019 22:24:59 -0300 Subject: [PATCH 3/8] Backport: Enable punctuations ending mentions (#8889) (#8894) * Enable punctuations ending mentions * Improve tests --- modules/references/references.go | 2 +- modules/references/references_test.go | 47 ++++++++++++++++++++------- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/modules/references/references.go b/modules/references/references.go index 9c74d0d081ae..2e36eecec501 100644 --- a/modules/references/references.go +++ b/modules/references/references.go @@ -26,7 +26,7 @@ var ( // TODO: fix invalid linking issue // mentionPattern matches all mentions in the form of "@user" - mentionPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(@[0-9a-zA-Z-_\.]+)(?:\s|$|\)|\])`) + mentionPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(@[0-9a-zA-Z-_]+|@[0-9a-zA-Z-_][0-9a-zA-Z-_.]+[0-9a-zA-Z-_])(?:\s|[:,;.?!]\s|[:,;.?!]?$|\)|\])`) // issueNumericPattern matches string that references to a numeric issue, e.g. #1287 issueNumericPattern = regexp.MustCompile(`(?:\s|^|\(|\[)(#[0-9]+)(?:\s|$|\)|\]|:|\.(\s|$))`) // issueAlphanumericPattern matches string that references to an alphanumeric issue, e.g. ABC-1234 diff --git a/modules/references/references_test.go b/modules/references/references_test.go index f8153ffe36da..23403b42baef 100644 --- a/modules/references/references_test.go +++ b/modules/references/references_test.go @@ -204,14 +204,32 @@ func TestFindAllIssueReferences(t *testing.T) { } func TestRegExp_mentionPattern(t *testing.T) { - trueTestCases := []string{ - "@Unknwon", - "@ANT_123", - "@xxx-DiN0-z-A..uru..s-xxx", - " @lol ", - " @Te-st", - "(@gitea)", - "[@gitea]", + trueTestCases := []struct { + pat string + exp string + }{ + {"@Unknwon", "@Unknwon"}, + {"@ANT_123", "@ANT_123"}, + {"@xxx-DiN0-z-A..uru..s-xxx", "@xxx-DiN0-z-A..uru..s-xxx"}, + {" @lol ", "@lol"}, + {" @Te-st", "@Te-st"}, + {"(@gitea)", "@gitea"}, + {"[@gitea]", "@gitea"}, + {"@gitea! this", "@gitea"}, + {"@gitea? this", "@gitea"}, + {"@gitea. this", "@gitea"}, + {"@gitea, this", "@gitea"}, + {"@gitea; this", "@gitea"}, + {"@gitea!\nthis", "@gitea"}, + {"\n@gitea?\nthis", "@gitea"}, + {"\t@gitea.\nthis", "@gitea"}, + {"@gitea,\nthis", "@gitea"}, + {"@gitea;\nthis", "@gitea"}, + {"@gitea!", "@gitea"}, + {"@gitea?", "@gitea"}, + {"@gitea.", "@gitea"}, + {"@gitea,", "@gitea"}, + {"@gitea;", "@gitea"}, } falseTestCases := []string{ "@ 0", @@ -219,17 +237,24 @@ func TestRegExp_mentionPattern(t *testing.T) { "@", "", "ABC", + "@.ABC", "/home/gitea/@gitea", "\"@gitea\"", + "@@gitea", + "@gitea!this", + "@gitea?this", + "@gitea,this", + "@gitea;this", } for _, testCase := range trueTestCases { - res := mentionPattern.MatchString(testCase) - assert.True(t, res) + found := mentionPattern.FindStringSubmatch(testCase.pat) + assert.Len(t, found, 2) + assert.Equal(t, testCase.exp, found[1]) } for _, testCase := range falseTestCases { res := mentionPattern.MatchString(testCase) - assert.False(t, res) + assert.False(t, res, "[%s] should be false", testCase) } } From 43fc99a7ed67152ff95af7c5ae3a7d7b131d567e Mon Sep 17 00:00:00 2001 From: mrsdizzie Date: Wed, 13 Nov 2019 00:15:57 -0500 Subject: [PATCH 4/8] Update Github Migration Tests (#8938) (#8945) Update all Github migration tests to use a new repo created just for these tests that won't accidentially be modified by regular users interacting with issues. Fixes #8895 --- modules/migrations/github_test.go | 421 ++++++++++-------------------- 1 file changed, 143 insertions(+), 278 deletions(-) diff --git a/modules/migrations/github_test.go b/modules/migrations/github_test.go index d9976d11326a..d2973b7b6c22 100644 --- a/modules/migrations/github_test.go +++ b/modules/migrations/github_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/assert" ) -func assertMilestoneEqual(t *testing.T, title, dueOn, created, updated, closed, state string, ms *base.Milestone) { +func assertMilestoneEqual(t *testing.T, description, title, dueOn, created, updated, closed, state string, ms *base.Milestone) { var tmPtr *time.Time if dueOn != "" { tm, err := time.Parse("2006-01-02 15:04:05 -0700 MST", dueOn) @@ -43,32 +43,34 @@ func assertMilestoneEqual(t *testing.T, title, dueOn, created, updated, closed, } assert.EqualValues(t, &base.Milestone{ - Title: title, - Deadline: tmPtr, - State: state, - Created: createdTM, - Updated: updatedTM, - Closed: closedTM, + Description: description, + Title: title, + Deadline: tmPtr, + State: state, + Created: createdTM, + Updated: updatedTM, + Closed: closedTM, }, ms) } -func assertLabelEqual(t *testing.T, name, color string, label *base.Label) { +func assertLabelEqual(t *testing.T, name, color, description string, label *base.Label) { assert.EqualValues(t, &base.Label{ - Name: name, - Color: color, + Name: name, + Color: color, + Description: description, }, label) } func TestGitHubDownloadRepo(t *testing.T) { - downloader := NewGithubDownloaderV3("", "", "go-gitea", "gitea") + downloader := NewGithubDownloaderV3("", "", "go-gitea", "test_repo") repo, err := downloader.GetRepoInfo() assert.NoError(t, err) assert.EqualValues(t, &base.Repository{ - Name: "gitea", + Name: "test_repo", Owner: "go-gitea", - Description: "Git with a cup of tea, painless self-hosted git service", - CloneURL: "https://github.com/go-gitea/gitea.git", - OriginalURL: "https://github.com/go-gitea/gitea", + Description: "Test repository for testing migration from github to gitea", + CloneURL: "https://github.com/go-gitea/test_repo.git", + OriginalURL: "https://github.com/go-gitea/test_repo", }, repo) topics, err := downloader.GetTopics() @@ -77,77 +79,46 @@ func TestGitHubDownloadRepo(t *testing.T) { milestones, err := downloader.GetMilestones() assert.NoError(t, err) - // before this tool release, we have 39 milestones on github.com/go-gitea/gitea - assert.True(t, len(milestones) >= 39) + assert.True(t, len(milestones) >= 2) for _, milestone := range milestones { switch milestone.Title { case "1.0.0": - assertMilestoneEqual(t, "1.0.0", "2016-12-23 08:00:00 +0000 UTC", - "2016-11-02 18:06:55 +0000 UTC", - "2016-12-29 10:26:00 +0000 UTC", - "2016-12-24 00:40:56 +0000 UTC", + assertMilestoneEqual(t, "Milestone 1.0.0", "1.0.0", "2019-11-11 08:00:00 +0000 UTC", + "2019-11-12 19:37:08 +0000 UTC", + "2019-11-12 21:56:17 +0000 UTC", + "2019-11-12 19:45:49 +0000 UTC", "closed", milestone) case "1.1.0": - assertMilestoneEqual(t, "1.1.0", "2017-02-24 08:00:00 +0000 UTC", - "2016-11-03 08:40:10 +0000 UTC", - "2017-06-15 05:04:36 +0000 UTC", - "2017-03-09 21:22:21 +0000 UTC", - "closed", milestone) - case "1.2.0": - assertMilestoneEqual(t, "1.2.0", "2017-04-24 07:00:00 +0000 UTC", - "2016-11-03 08:40:15 +0000 UTC", - "2017-12-10 02:43:29 +0000 UTC", - "2017-10-12 08:24:28 +0000 UTC", - "closed", milestone) - case "1.3.0": - assertMilestoneEqual(t, "1.3.0", "2017-11-29 08:00:00 +0000 UTC", - "2017-03-03 08:08:59 +0000 UTC", - "2017-12-04 07:48:44 +0000 UTC", - "2017-11-29 18:39:00 +0000 UTC", - "closed", milestone) - case "1.4.0": - assertMilestoneEqual(t, "1.4.0", "2018-01-25 08:00:00 +0000 UTC", - "2017-08-23 11:02:37 +0000 UTC", - "2018-03-25 20:01:56 +0000 UTC", - "2018-03-25 20:01:56 +0000 UTC", - "closed", milestone) - case "1.5.0": - assertMilestoneEqual(t, "1.5.0", "2018-06-15 07:00:00 +0000 UTC", - "2017-12-30 04:21:56 +0000 UTC", - "2018-09-05 16:34:22 +0000 UTC", - "2018-08-11 08:45:01 +0000 UTC", - "closed", milestone) - case "1.7.0": - assertMilestoneEqual(t, "1.7.0", "2018-12-25 08:00:00 +0000 UTC", - "2018-08-28 14:20:14 +0000 UTC", - "2019-01-27 11:30:24 +0000 UTC", - "2019-01-23 08:58:23 +0000 UTC", + assertMilestoneEqual(t, "Milestone 1.1.0", "1.1.0", "2019-11-12 08:00:00 +0000 UTC", + "2019-11-12 19:37:25 +0000 UTC", + "2019-11-12 21:39:27 +0000 UTC", + "2019-11-12 19:45:46 +0000 UTC", "closed", milestone) } } labels, err := downloader.GetLabels() assert.NoError(t, err) - assert.True(t, len(labels) >= 48) + assert.True(t, len(labels) >= 8) for _, l := range labels { switch l.Name { - case "backport/v1.7": - assertLabelEqual(t, "backport/v1.7", "fbca04", l) - case "backport/v1.8": - assertLabelEqual(t, "backport/v1.8", "fbca04", l) - case "kind/api": - assertLabelEqual(t, "kind/api", "5319e7", l) - case "kind/breaking": - assertLabelEqual(t, "kind/breaking", "fbca04", l) - case "kind/bug": - assertLabelEqual(t, "kind/bug", "ee0701", l) - case "kind/docs": - assertLabelEqual(t, "kind/docs", "c2e0c6", l) - case "kind/enhancement": - assertLabelEqual(t, "kind/enhancement", "84b6eb", l) - case "kind/feature": - assertLabelEqual(t, "kind/feature", "006b75", l) + case "bug": + assertLabelEqual(t, "bug", "d73a4a", "Something isn't working", l) + case "documentation": + assertLabelEqual(t, "documentation", "0075ca", "Improvements or additions to documentation", l) + case "duplicate": + assertLabelEqual(t, "duplicate", "cfd3d7", "This issue or pull request already exists", l) + case "enhancement": + assertLabelEqual(t, "enhancement", "a2eeef", "New feature or request", l) + case "good first issue": + assertLabelEqual(t, "good first issue", "7057ff", "Good for newcomers", l) + case "help wanted": + assertLabelEqual(t, "help wanted", "008672", "Extra attention is needed", l) + case "invalid": + assertLabelEqual(t, "invalid", "e4e669", "This doesn't seem right", l) + case "question": + assertLabelEqual(t, "question", "d876e3", "Further information is requested", l) } } @@ -157,48 +128,50 @@ func TestGitHubDownloadRepo(t *testing.T) { { TagName: "v0.9.99", TargetCommitish: "master", - Name: "fork", - Body: "Forked source from Gogs into Gitea\n", - Created: time.Date(2016, 10, 17, 02, 17, 59, 0, time.UTC), - Published: time.Date(2016, 11, 17, 15, 37, 0, 0, time.UTC), - PublisherID: 4726179, - PublisherName: "bkcsoft", + Name: "First Release", + Body: "A test release", + Created: time.Date(2019, 11, 9, 16, 49, 21, 0, time.UTC), + Published: time.Date(2019, 11, 12, 20, 12, 10, 0, time.UTC), + PublisherID: 1669571, + PublisherName: "mrsdizzie", }, }, releases[len(releases)-1:]) // downloader.GetIssues() - issues, isEnd, err := downloader.GetIssues(1, 8) + issues, isEnd, err := downloader.GetIssues(1, 2) assert.NoError(t, err) - assert.EqualValues(t, 3, len(issues)) + assert.EqualValues(t, 2, len(issues)) assert.False(t, isEnd) var ( - closed1 = time.Date(2018, 10, 23, 02, 57, 43, 0, time.UTC) - closed7 = time.Date(2019, 7, 8, 8, 20, 23, 0, time.UTC) + closed1 = time.Date(2019, 11, 12, 20, 22, 22, 0, time.UTC) + closed2 = time.Date(2019, 11, 12, 21, 1, 31, 0, time.UTC) ) assert.EqualValues(t, []*base.Issue{ { - Number: 6, - Title: "Contribution system: History heatmap for user", - Content: "Hi guys,\r\n\r\nI think that is a possible feature, a history heatmap similar to github or gitlab.\r\nActually exists a plugin called Calendar HeatMap. I used this on mine project to heat application log and worked fine here.\r\nThen, is only a idea, what you think? :)\r\n\r\nhttp://cal-heatmap.com/\r\nhttps://github.com/wa0x6e/cal-heatmap\r\n\r\nReference: https://github.com/gogits/gogs/issues/1640", - Milestone: "1.7.0", - PosterID: 1520407, - PosterName: "joubertredrat", + Number: 1, + Title: "Please add an animated gif icon to the merge button", + Content: "I just want the merge button to hurt my eyes a little. \xF0\x9F\x98\x9D ", + Milestone: "1.0.0", + PosterID: 18600385, + PosterName: "guillep2k", State: "closed", - Created: time.Date(2016, 11, 02, 18, 51, 55, 0, time.UTC), + Created: time.Date(2019, 11, 9, 17, 0, 29, 0, time.UTC), Labels: []*base.Label{ { - Name: "kind/feature", - Color: "006b75", + Name: "bug", + Color: "d73a4a", + Description: "Something isn't working", }, { - Name: "kind/ui", - Color: "fef2c0", + Name: "good first issue", + Color: "7057ff", + Description: "Good for newcomers", }, }, Reactions: &base.Reactions{ - TotalCount: 0, - PlusOne: 0, + TotalCount: 1, + PlusOne: 1, MinusOne: 0, Laugh: 0, Confused: 0, @@ -208,84 +181,48 @@ func TestGitHubDownloadRepo(t *testing.T) { Closed: &closed1, }, { - Number: 7, - Title: "display page revisions on wiki", - Content: "Hi guys,\r\n\r\nWiki on Gogs is very fine, I liked a lot, but I think that is good idea to be possible see other revisions from page as a page history.\r\n\r\nWhat you think?\r\n\r\nReference: https://github.com/gogits/gogs/issues/2991", - Milestone: "1.10.0", - PosterID: 1520407, - PosterName: "joubertredrat", + Number: 2, + Title: "Test issue", + Content: "This is test issue 2, do not touch!", + Milestone: "1.1.0", + PosterID: 1669571, + PosterName: "mrsdizzie", State: "closed", - Created: time.Date(2016, 11, 02, 18, 57, 32, 0, time.UTC), + Created: time.Date(2019, 11, 12, 21, 0, 6, 0, time.UTC), Labels: []*base.Label{ { - Name: "kind/feature", - Color: "006b75", - }, - { - Name: "reviewed/confirmed", - Color: "8d9b12", - Description: "Issue has been reviewed and confirmed to be present or accepted to be implemented", + Name: "duplicate", + Color: "cfd3d7", + Description: "This issue or pull request already exists", }, }, Reactions: &base.Reactions{ TotalCount: 6, - PlusOne: 5, - MinusOne: 0, - Laugh: 0, + PlusOne: 1, + MinusOne: 1, + Laugh: 1, Confused: 1, - Heart: 0, - Hooray: 0, - }, - Closed: &closed7, - }, - { - Number: 8, - Title: "audit logs", - Content: "Hi,\r\n\r\nI think that is good idea to have user operation log to admin see what the user is doing at Gogs. Similar to example below\r\n\r\n| user | operation | information |\r\n| --- | --- | --- |\r\n| joubertredrat | repo.create | Create repo MyProjectData |\r\n| joubertredrat | user.settings | Edit settings |\r\n| tboerger | repo.fork | Create Fork from MyProjectData to ForkMyProjectData |\r\n| bkcsoft | repo.remove | Remove repo MySource |\r\n| tboerger | admin.auth | Edit auth LDAP org-connection |\r\n\r\nThis resource can be used on user page too, as user activity, set that log row is public (repo._) or private (user._, admin.*) and display only public activity.\r\n\r\nWhat you think?\r\n\r\n[Chat summary from March 14, 2017](https://github.com/go-gitea/gitea/issues/8#issuecomment-286463807)\r\n\r\nReferences:\r\nhttps://github.com/gogits/gogs/issues/3016", - Milestone: "1.x.x", - PosterID: 1520407, - PosterName: "joubertredrat", - State: "open", - Created: time.Date(2016, 11, 02, 18, 59, 20, 0, time.UTC), - Labels: []*base.Label{ - { - Name: "kind/feature", - Color: "006b75", - }, - { - Name: "kind/proposal", - Color: "5319e7", - }, - }, - Reactions: &base.Reactions{ - TotalCount: 9, - PlusOne: 8, - MinusOne: 0, - Laugh: 0, - Confused: 0, Heart: 1, - Hooray: 0, + Hooray: 1, }, + Closed: &closed2, }, }, issues) // downloader.GetComments() - comments, err := downloader.GetComments(6) + comments, err := downloader.GetComments(2) assert.NoError(t, err) - assert.EqualValues(t, 35, len(comments)) + assert.EqualValues(t, 2, len(comments)) assert.EqualValues(t, []*base.Comment{ { - IssueIndex: 6, - PosterID: 4726179, - PosterName: "bkcsoft", - Created: time.Date(2016, 11, 02, 18, 59, 48, 0, time.UTC), - Content: `I would prefer a solution that is in the backend, unless it's required to have it update without reloading. Unfortunately I can't seem to find anything that does that :unamused: - -Also this would _require_ caching, since it will fetch huge amounts of data from disk... -`, + IssueIndex: 2, + PosterID: 1669571, + PosterName: "mrsdizzie", + Created: time.Date(2019, 11, 12, 21, 0, 13, 0, time.UTC), + Content: "This is a comment", Reactions: &base.Reactions{ - TotalCount: 2, - PlusOne: 2, + TotalCount: 1, + PlusOne: 1, MinusOne: 0, Laugh: 0, Confused: 0, @@ -294,14 +231,11 @@ Also this would _require_ caching, since it will fetch huge amounts of data from }, }, { - IssueIndex: 6, - PosterID: 1520407, - PosterName: "joubertredrat", - Created: time.Date(2016, 11, 02, 19, 16, 56, 0, time.UTC), - Content: `Yes, this plugin build on front-end, with backend I don't know too, but we can consider make component for this. - -In my case I use ajax to get data, but build on frontend anyway -`, + IssueIndex: 2, + PosterID: 1669571, + PosterName: "mrsdizzie", + Created: time.Date(2019, 11, 12, 22, 7, 14, 0, time.UTC), + Content: "A second comment", Reactions: &base.Reactions{ TotalCount: 0, PlusOne: 0, @@ -312,154 +246,85 @@ In my case I use ajax to get data, but build on frontend anyway Hooray: 0, }, }, - { - IssueIndex: 6, - PosterID: 1799009, - PosterName: "xinity", - Created: time.Date(2016, 11, 03, 13, 04, 56, 0, time.UTC), - Content: `following @bkcsoft retention strategy in cache is a must if we don't want gitea to waste ressources. -something like in the latest 15days could be enough don't you think ? -`, - Reactions: &base.Reactions{ - TotalCount: 2, - PlusOne: 2, - MinusOne: 0, - Laugh: 0, - Confused: 0, - Heart: 0, - Hooray: 0, - }, - }, - }, comments[:3]) + }, comments[:2]) // downloader.GetPullRequests() - prs, err := downloader.GetPullRequests(1, 3) + prs, err := downloader.GetPullRequests(1, 2) assert.NoError(t, err) - assert.EqualValues(t, 3, len(prs)) + assert.EqualValues(t, 2, len(prs)) - closed1 = time.Date(2016, 11, 02, 18, 22, 21, 0, time.UTC) - var ( - closed2 = time.Date(2016, 11, 03, 8, 06, 27, 0, time.UTC) - closed3 = time.Date(2016, 11, 02, 18, 22, 31, 0, time.UTC) - ) + closed1 = time.Date(2019, 11, 12, 21, 39, 27, 0, time.UTC) + var merged1 = time.Date(2019, 11, 12, 21, 39, 27, 0, time.UTC) - var ( - merged1 = time.Date(2016, 11, 02, 18, 22, 21, 0, time.UTC) - merged2 = time.Date(2016, 11, 03, 8, 06, 27, 0, time.UTC) - merged3 = time.Date(2016, 11, 02, 18, 22, 31, 0, time.UTC) - ) assert.EqualValues(t, []*base.PullRequest{ { - Number: 1, - Title: "Rename import paths: \"github.com/gogits/gogs\" -> \"github.com/go-gitea/gitea\"", - Content: "", - Milestone: "1.0.0", - PosterID: 7011819, - PosterName: "andreynering", + Number: 3, + Title: "Update README.md", + Content: "add warning to readme", + Milestone: "1.1.0", + PosterID: 1669571, + PosterName: "mrsdizzie", State: "closed", - Created: time.Date(2016, 11, 02, 17, 01, 19, 0, time.UTC), + Created: time.Date(2019, 11, 12, 21, 21, 43, 0, time.UTC), Labels: []*base.Label{ { - Name: "kind/enhancement", - Color: "84b6eb", - }, - { - Name: "lgtm/done", - Color: "0e8a16", + Name: "documentation", + Color: "0075ca", + Description: "Improvements or additions to documentation", }, }, - PatchURL: "https://github.com/go-gitea/gitea/pull/1.patch", + PatchURL: "https://github.com/go-gitea/test_repo/pull/3.patch", Head: base.PullRequestBranch{ - Ref: "import-paths", - SHA: "1b0ec3208db8501acba44a137c009a5a126ebaa9", - OwnerName: "andreynering", + Ref: "master", + CloneURL: "https://github.com/mrsdizzie/test_repo.git", + SHA: "076160cf0b039f13e5eff19619932d181269414b", + RepoName: "test_repo", + + OwnerName: "mrsdizzie", }, Base: base.PullRequestBranch{ Ref: "master", - SHA: "6bcff7828f117af8d51285ce3acba01a7e40a867", + SHA: "72866af952e98d02a73003501836074b286a78f6", OwnerName: "go-gitea", - RepoName: "gitea", + RepoName: "test_repo", }, Closed: &closed1, Merged: true, MergedTime: &merged1, - MergeCommitSHA: "142d35e8d2baec230ddb565d1265940d59141fab", - }, - { - Number: 2, - Title: "Fix sender of issue notifications", - Content: "It is the FROM field in mailer configuration that needs be used,\r\nnot the USER field, which is for authentication.\r\n\r\nMigrated from https://github.com/gogits/gogs/pull/3616\r\n", - Milestone: "1.0.0", - PosterID: 289678, - PosterName: "strk", - State: "closed", - Created: time.Date(2016, 11, 02, 17, 24, 19, 0, time.UTC), - Labels: []*base.Label{ - { - Name: "kind/bug", - Color: "ee0701", - }, - { - Name: "lgtm/done", - Color: "0e8a16", - }, - }, - PatchURL: "https://github.com/go-gitea/gitea/pull/2.patch", - Head: base.PullRequestBranch{ - Ref: "proper-from-on-issue-mail", - SHA: "af03d00780a6ee70c58e135c6679542cde4f8d50", - RepoName: "gogs", - OwnerName: "strk", - CloneURL: "https://github.com/strk/gogs.git", - }, - Base: base.PullRequestBranch{ - Ref: "develop", - SHA: "5c5424301443ffa3659737d12de48ab1dfe39a00", - OwnerName: "go-gitea", - RepoName: "gitea", - }, - Closed: &closed2, - Merged: true, - MergedTime: &merged2, - MergeCommitSHA: "d8de2beb5b92d02a0597ba7c7803839380666653", + MergeCommitSHA: "f32b0a9dfd09a60f616f29158f772cedd89942d2", }, { - Number: 3, - Title: "Use proper url for libravatar dep", - Content: "Fetch go-libravatar from its official source, rather than from an unmaintained fork\r\n", + Number: 4, + Title: "Test branch", + Content: "do not merge this PR", Milestone: "1.0.0", - PosterID: 289678, - PosterName: "strk", - State: "closed", - Created: time.Date(2016, 11, 02, 17, 34, 31, 0, time.UTC), + PosterID: 1669571, + PosterName: "mrsdizzie", + State: "open", + Created: time.Date(2019, 11, 12, 21, 54, 18, 0, time.UTC), Labels: []*base.Label{ { - Name: "kind/enhancement", - Color: "84b6eb", - }, - { - Name: "lgtm/done", - Color: "0e8a16", + Name: "bug", + Color: "d73a4a", + Description: "Something isn't working", }, }, - PatchURL: "https://github.com/go-gitea/gitea/pull/3.patch", + PatchURL: "https://github.com/go-gitea/test_repo/pull/4.patch", Head: base.PullRequestBranch{ - Ref: "libravatar-proper-url", - SHA: "d59a48a2550abd4129b96d38473941b895a4859b", - RepoName: "gogs", - OwnerName: "strk", - CloneURL: "https://github.com/strk/gogs.git", + Ref: "test-branch", + SHA: "2be9101c543658591222acbee3eb799edfc3853d", + RepoName: "test_repo", + OwnerName: "mrsdizzie", + CloneURL: "https://github.com/mrsdizzie/test_repo.git", }, Base: base.PullRequestBranch{ - Ref: "develop", - SHA: "6bcff7828f117af8d51285ce3acba01a7e40a867", + Ref: "master", + SHA: "f32b0a9dfd09a60f616f29158f772cedd89942d2", OwnerName: "go-gitea", - RepoName: "gitea", + RepoName: "test_repo", }, - Closed: &closed3, - Merged: true, - MergedTime: &merged3, - MergeCommitSHA: "5c5424301443ffa3659737d12de48ab1dfe39a00", + Merged: false, + MergeCommitSHA: "565d1208f5fffdc1c5ae1a2436491eb9a5e4ebae", }, }, prs) } From 3497efac4ad74e766da6a6eaaca8dddec409040a Mon Sep 17 00:00:00 2001 From: zeripath Date: Wed, 13 Nov 2019 13:54:04 +0000 Subject: [PATCH 5/8] Add Close() method to gogitRepository (#8901) (#8956) Backport #8901 In investigating #7947 it has become clear that the storage component of go-git repositories needs closing. This PR adds this Close function and adds the Close functions as necessary. In TransferOwnership the ctx.Repo.GitRepo is closed if it is open to help prevent the risk of multiple open files. Fixes #7947 --- cmd/admin.go | 3 + docs/content/doc/advanced/migrations.en-us.md | 3 +- integrations/api_releases_test.go | 2 + integrations/api_repo_file_create_test.go | 1 + integrations/api_repo_file_update_test.go | 1 + .../api_repo_get_contents_list_test.go | 2 + integrations/api_repo_get_contents_test.go | 2 + integrations/api_repo_git_tags_test.go | 2 + integrations/repofiles_delete_test.go | 5 + integrations/repofiles_update_test.go | 18 +++ models/graph_test.go | 1 + models/migrations/v39.go | 1 + models/migrations/v82.go | 1 + models/pull.go | 5 + models/repo.go | 1 + models/repo_activity.go | 4 + models/repo_branch.go | 4 + models/repo_tag.go | 24 --- models/wiki.go | 2 + models/wiki_test.go | 3 + modules/context/api.go | 9 ++ modules/context/repo.go | 19 +++ modules/git/blame.go | 3 +- modules/git/blob_test.go | 4 + modules/git/commit_info_test.go | 10 +- modules/git/notes_test.go | 2 + modules/git/repo.go | 16 ++ modules/git/repo_blob_test.go | 3 + modules/git/repo_branch.go | 1 + modules/git/repo_branch_test.go | 2 + modules/git/repo_commit_test.go | 3 + modules/git/repo_compare_test.go | 1 + modules/git/repo_ref_test.go | 2 + modules/git/repo_stats_test.go | 1 + modules/git/repo_tag_test.go | 3 + modules/git/repo_test.go | 1 + modules/git/tree_entry_test.go | 1 + modules/migrations/base/uploader.go | 1 + modules/migrations/gitea.go | 7 + modules/migrations/migrate.go | 1 + modules/repofiles/action.go | 6 + modules/repofiles/blob.go | 1 + modules/repofiles/blob_test.go | 2 + modules/repofiles/commit_status.go | 2 + modules/repofiles/content.go | 2 + modules/repofiles/content_test.go | 12 ++ modules/repofiles/diff_test.go | 4 + modules/repofiles/file_test.go | 3 + modules/repofiles/temp_repo.go | 1 + modules/repofiles/tree.go | 1 + modules/repofiles/tree_test.go | 2 + modules/repofiles/update.go | 1 + modules/test/context_tests.go | 1 + routers/api/v1/api.go | 2 +- routers/api/v1/repo/commits.go | 2 + routers/api/v1/repo/file.go | 1 + routers/api/v1/repo/git_ref.go | 2 + routers/api/v1/repo/pull.go | 7 + routers/api/v1/repo/tag.go | 2 +- routers/repo/compare.go | 8 + routers/repo/editor_test.go | 5 + routers/repo/pull.go | 6 + routers/repo/release_test.go | 1 + routers/repo/setting.go | 10 ++ routers/repo/wiki.go | 70 +++++++- routers/repo/wiki_test.go | 1 + services/comments/comments.go | 1 + services/gitdiff/gitdiff.go | 2 + services/mirror/mirror.go | 152 +++++++++--------- services/mirror/mirror_test.go | 1 + services/pull/commit_status.go | 1 + services/release/release_test.go | 1 + 72 files changed, 386 insertions(+), 102 deletions(-) delete mode 100644 models/repo_tag.go diff --git a/cmd/admin.go b/cmd/admin.go index 4346159febf8..32ec665ff696 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -375,17 +375,20 @@ func runRepoSyncReleases(c *cli.Context) error { if err = models.SyncReleasesWithTags(repo, gitRepo); err != nil { log.Warn(" SyncReleasesWithTags: %v", err) + gitRepo.Close() continue } count, err = getReleaseCount(repo.ID) if err != nil { log.Warn(" GetReleaseCountByRepoID: %v", err) + gitRepo.Close() continue } log.Trace(" repo %s releases synchronized to tags: from %d to %d", repo.FullName(), oldnum, count) + gitRepo.Close() } } diff --git a/docs/content/doc/advanced/migrations.en-us.md b/docs/content/doc/advanced/migrations.en-us.md index 2511f7af8985..0d9d8b49a77b 100644 --- a/docs/content/doc/advanced/migrations.en-us.md +++ b/docs/content/doc/advanced/migrations.en-us.md @@ -68,6 +68,7 @@ type Uploader interface { CreateComment(issueNumber int64, comment *Comment) error CreatePullRequest(pr *PullRequest) error Rollback() error + Close() } -``` \ No newline at end of file +``` diff --git a/integrations/api_releases_test.go b/integrations/api_releases_test.go index 897f863eb3e0..8025f1de5d59 100644 --- a/integrations/api_releases_test.go +++ b/integrations/api_releases_test.go @@ -51,6 +51,7 @@ func TestAPICreateAndUpdateRelease(t *testing.T) { gitRepo, err := git.OpenRepository(repo.RepoPath()) assert.NoError(t, err) + defer gitRepo.Close() err = gitRepo.CreateTag("v0.0.1", "master") assert.NoError(t, err) @@ -112,6 +113,7 @@ func TestAPICreateReleaseToDefaultBranchOnExistingTag(t *testing.T) { gitRepo, err := git.OpenRepository(repo.RepoPath()) assert.NoError(t, err) + defer gitRepo.Close() err = gitRepo.CreateTag("v0.0.1", "master") assert.NoError(t, err) diff --git a/integrations/api_repo_file_create_test.go b/integrations/api_repo_file_create_test.go index 42898bf259fa..948d3b6f1cc5 100644 --- a/integrations/api_repo_file_create_test.go +++ b/integrations/api_repo_file_create_test.go @@ -139,6 +139,7 @@ func TestAPICreateFile(t *testing.T) { assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, fileResponse.Commit.HTMLURL) assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, fileResponse.Commit.Author.Email) assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, fileResponse.Commit.Author.Name) + gitRepo.Close() } // Test creating a file in a new branch diff --git a/integrations/api_repo_file_update_test.go b/integrations/api_repo_file_update_test.go index 366eb5e91895..c19d8d95829c 100644 --- a/integrations/api_repo_file_update_test.go +++ b/integrations/api_repo_file_update_test.go @@ -143,6 +143,7 @@ func TestAPIUpdateFile(t *testing.T) { assert.EqualValues(t, expectedFileResponse.Commit.HTMLURL, fileResponse.Commit.HTMLURL) assert.EqualValues(t, expectedFileResponse.Commit.Author.Email, fileResponse.Commit.Author.Email) assert.EqualValues(t, expectedFileResponse.Commit.Author.Name, fileResponse.Commit.Author.Name) + gitRepo.Close() } // Test updating a file in a new branch diff --git a/integrations/api_repo_get_contents_list_test.go b/integrations/api_repo_get_contents_list_test.go index f74ceb514a0b..4605ccf4d933 100644 --- a/integrations/api_repo_get_contents_list_test.go +++ b/integrations/api_repo_get_contents_list_test.go @@ -74,6 +74,8 @@ func testAPIGetContentsList(t *testing.T, u *url.URL) { repo1.CreateNewBranch(user2, repo1.DefaultBranch, newBranch) // Get the commit ID of the default branch gitRepo, _ := git.OpenRepository(repo1.RepoPath()) + defer gitRepo.Close() + commitID, _ := gitRepo.GetBranchCommitID(repo1.DefaultBranch) // Make a new tag in repo1 newTag := "test_tag" diff --git a/integrations/api_repo_get_contents_test.go b/integrations/api_repo_get_contents_test.go index f6a43bc5c63f..77a827ec6125 100644 --- a/integrations/api_repo_get_contents_test.go +++ b/integrations/api_repo_get_contents_test.go @@ -75,6 +75,8 @@ func testAPIGetContents(t *testing.T, u *url.URL) { repo1.CreateNewBranch(user2, repo1.DefaultBranch, newBranch) // Get the commit ID of the default branch gitRepo, _ := git.OpenRepository(repo1.RepoPath()) + defer gitRepo.Close() + commitID, _ := gitRepo.GetBranchCommitID(repo1.DefaultBranch) // Make a new tag in repo1 newTag := "test_tag" diff --git a/integrations/api_repo_git_tags_test.go b/integrations/api_repo_git_tags_test.go index ae519249e03b..d6ff08990a37 100644 --- a/integrations/api_repo_git_tags_test.go +++ b/integrations/api_repo_git_tags_test.go @@ -29,6 +29,8 @@ func TestAPIGitTags(t *testing.T) { git.NewCommand("config", "user.email", user.Email).RunInDir(repo.RepoPath()) gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() + commit, _ := gitRepo.GetBranchCommit("master") lTagName := "lightweightTag" gitRepo.CreateTag(lTagName, commit.ID.String()) diff --git a/integrations/repofiles_delete_test.go b/integrations/repofiles_delete_test.go index f4cb4510be44..10742bf0b957 100644 --- a/integrations/repofiles_delete_test.go +++ b/integrations/repofiles_delete_test.go @@ -73,6 +73,7 @@ func testDeleteRepoFile(t *testing.T, u *url.URL) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() repo := ctx.Repo.Repository doer := ctx.User opts := getDeleteRepoFileOptions(repo) @@ -111,6 +112,8 @@ func testDeleteRepoFileWithoutBranchNames(t *testing.T, u *url.URL) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User opts := getDeleteRepoFileOptions(repo) @@ -139,6 +142,8 @@ func TestDeleteRepoFileErrors(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User diff --git a/integrations/repofiles_update_test.go b/integrations/repofiles_update_test.go index a4ce16d8479e..5b88bb819833 100644 --- a/integrations/repofiles_update_test.go +++ b/integrations/repofiles_update_test.go @@ -191,6 +191,8 @@ func TestCreateOrUpdateRepoFileForCreate(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User opts := getCreateRepoFileOptions(repo) @@ -201,6 +203,8 @@ func TestCreateOrUpdateRepoFileForCreate(t *testing.T) { // asserts assert.Nil(t, err) gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() + commitID, _ := gitRepo.GetBranchCommitID(opts.NewBranch) expectedFileResponse := getExpectedFileResponseForRepofilesCreate(commitID) assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content) @@ -220,6 +224,8 @@ func TestCreateOrUpdateRepoFileForUpdate(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User opts := getUpdateRepoFileOptions(repo) @@ -230,6 +236,8 @@ func TestCreateOrUpdateRepoFileForUpdate(t *testing.T) { // asserts assert.Nil(t, err) gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() + commitID, _ := gitRepo.GetBranchCommitID(opts.NewBranch) expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commitID, opts.TreePath) assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content) @@ -249,6 +257,8 @@ func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User opts := getUpdateRepoFileOptions(repo) @@ -261,6 +271,8 @@ func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) { // asserts assert.Nil(t, err) gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() + commit, _ := gitRepo.GetBranchCommit(opts.NewBranch) expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commit.ID.String(), opts.TreePath) // assert that the old file no longer exists in the last commit of the branch @@ -288,6 +300,8 @@ func TestCreateOrUpdateRepoFileWithoutBranchNames(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User opts := getUpdateRepoFileOptions(repo) @@ -300,6 +314,8 @@ func TestCreateOrUpdateRepoFileWithoutBranchNames(t *testing.T) { // asserts assert.Nil(t, err) gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() + commitID, _ := gitRepo.GetBranchCommitID(repo.DefaultBranch) expectedFileResponse := getExpectedFileResponseForRepofilesUpdate(commitID, opts.TreePath) assert.EqualValues(t, expectedFileResponse.Content, fileResponse.Content) @@ -315,6 +331,8 @@ func TestCreateOrUpdateRepoFileErrors(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository doer := ctx.User diff --git a/models/graph_test.go b/models/graph_test.go index 5c78e3877b57..4e104592a3a1 100644 --- a/models/graph_test.go +++ b/models/graph_test.go @@ -17,6 +17,7 @@ func BenchmarkGetCommitGraph(b *testing.B) { if err != nil { b.Error("Could not open repository") } + defer currentRepo.Close() for i := 0; i < b.N; i++ { graph, err := GetCommitGraph(currentRepo) diff --git a/models/migrations/v39.go b/models/migrations/v39.go index 1312cb33134b..992e06bcaae1 100644 --- a/models/migrations/v39.go +++ b/models/migrations/v39.go @@ -47,6 +47,7 @@ func releaseAddColumnIsTagAndSyncTags(x *xorm.Engine) error { if err = models.SyncReleasesWithTags(repo, gitRepo); err != nil { log.Warn("SyncReleasesWithTags: %v", err) } + gitRepo.Close() } if len(repos) < pageSize { break diff --git a/models/migrations/v82.go b/models/migrations/v82.go index eb73f1834371..83da703c8a0f 100644 --- a/models/migrations/v82.go +++ b/models/migrations/v82.go @@ -91,6 +91,7 @@ func fixReleaseSha1OnReleaseTable(x *xorm.Engine) error { if err != nil { return err } + defer gitRepo.Close() gitRepoCache[release.RepoID] = gitRepo } diff --git a/models/pull.go b/models/pull.go index 2016874574dc..f93257008be4 100644 --- a/models/pull.go +++ b/models/pull.go @@ -382,6 +382,7 @@ func (pr *PullRequest) GetLastCommitStatus() (status *CommitStatus, err error) { if err != nil { return nil, err } + defer headGitRepo.Close() lastCommitID, err := headGitRepo.GetBranchCommitID(pr.HeadBranch) if err != nil { @@ -571,6 +572,7 @@ func (pr *PullRequest) getMergeCommit() (*git.Commit, error) { if err != nil { return nil, fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() commit, err := gitRepo.GetCommit(mergeCommit[:40]) if err != nil { @@ -955,6 +957,7 @@ func (pr *PullRequest) UpdatePatch() (err error) { if err != nil { return fmt.Errorf("OpenRepository: %v", err) } + defer headGitRepo.Close() // Add a temporary remote. tmpRemote := com.ToStr(time.Now().UnixNano()) @@ -996,6 +999,7 @@ func (pr *PullRequest) PushToBaseRepo() (err error) { if err != nil { return fmt.Errorf("OpenRepository: %v", err) } + defer headGitRepo.Close() tmpRemoteName := fmt.Sprintf("tmp-pull-%d", pr.ID) if err = headGitRepo.AddRemote(tmpRemoteName, pr.BaseRepo.RepoPath(), false); err != nil { @@ -1185,6 +1189,7 @@ func checkForInvalidation(requests PullRequestList, repoID int64, doer *User, br if err != nil { log.Error("PullRequestList.InvalidateCodeComments: %v", err) } + gitRepo.Close() }() return nil } diff --git a/models/repo.go b/models/repo.go index d8a462c37bc0..8875c963761c 100644 --- a/models/repo.go +++ b/models/repo.go @@ -998,6 +998,7 @@ func MigrateRepositoryGitData(doer, u *User, repo *Repository, opts api.MigrateR if err != nil { return repo, fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() repo.IsEmpty, err = gitRepo.IsEmpty() if err != nil { diff --git a/models/repo_activity.go b/models/repo_activity.go index 04612ae1efca..c95af85495b7 100644 --- a/models/repo_activity.go +++ b/models/repo_activity.go @@ -64,6 +64,8 @@ func GetActivityStats(repo *Repository, timeFrom time.Time, releases, issues, pr if err != nil { return nil, fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() + code, err := gitRepo.GetCodeActivityStats(timeFrom, repo.DefaultBranch) if err != nil { return nil, fmt.Errorf("FillFromGit: %v", err) @@ -79,6 +81,8 @@ func GetActivityStatsTopAuthors(repo *Repository, timeFrom time.Time, count int) if err != nil { return nil, fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() + code, err := gitRepo.GetCodeActivityStats(timeFrom, "") if err != nil { return nil, fmt.Errorf("FillFromGit: %v", err) diff --git a/models/repo_branch.go b/models/repo_branch.go index dee6ef3d7e26..c51323183688 100644 --- a/models/repo_branch.go +++ b/models/repo_branch.go @@ -23,6 +23,7 @@ func (repo *Repository) GetBranch(branch string) (*git.Branch, error) { if err != nil { return nil, err } + defer gitRepo.Close() return gitRepo.GetBranch(branch) } @@ -38,6 +39,7 @@ func (repo *Repository) CheckBranchName(name string) error { if err != nil { return err } + defer gitRepo.Close() branches, err := repo.GetBranches() if err != nil { @@ -94,6 +96,7 @@ func (repo *Repository) CreateNewBranch(doer *User, oldBranchName, branchName st log.Error("Unable to open temporary repository: %s (%v)", basePath, err) return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err) } + defer gitRepo.Close() if err = gitRepo.CreateBranch(branchName, oldBranchName); err != nil { log.Error("Unable to create branch: %s from %s. (%v)", branchName, oldBranchName, err) @@ -140,6 +143,7 @@ func (repo *Repository) CreateNewBranchFromCommit(doer *User, commit, branchName log.Error("Unable to open temporary repository: %s (%v)", basePath, err) return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err) } + defer gitRepo.Close() if err = gitRepo.CreateBranch(branchName, commit); err != nil { log.Error("Unable to create branch: %s from %s. (%v)", branchName, commit, err) diff --git a/models/repo_tag.go b/models/repo_tag.go deleted file mode 100644 index 3864b7a12a7c..000000000000 --- a/models/repo_tag.go +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2019 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -package models - -import ( - "code.gitea.io/gitea/modules/git" -) - -// GetTagsByPath returns repo tags by its path -func GetTagsByPath(path string) ([]*git.Tag, error) { - gitRepo, err := git.OpenRepository(path) - if err != nil { - return nil, err - } - - return gitRepo.GetTagInfos() -} - -// GetTags return repo's tags -func (repo *Repository) GetTags() ([]*git.Tag, error) { - return GetTagsByPath(repo.RepoPath()) -} diff --git a/models/wiki.go b/models/wiki.go index 0460e0f07944..8a2f739dadff 100644 --- a/models/wiki.go +++ b/models/wiki.go @@ -140,6 +140,7 @@ func (repo *Repository) updateWikiPage(doer *User, oldWikiName, newWikiName, con log.Error("Unable to open temporary repository: %s (%v)", basePath, err) return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err) } + defer gitRepo.Close() if hasMasterBranch { if err := gitRepo.ReadTreeToIndex("HEAD"); err != nil { @@ -276,6 +277,7 @@ func (repo *Repository) DeleteWikiPage(doer *User, wikiName string) (err error) log.Error("Unable to open temporary repository: %s (%v)", basePath, err) return fmt.Errorf("Failed to open new temporary repository in: %s %v", basePath, err) } + defer gitRepo.Close() if err := gitRepo.ReadTreeToIndex("HEAD"); err != nil { log.Error("Unable to read HEAD tree to index in: %s %v", basePath, err) diff --git a/models/wiki_test.go b/models/wiki_test.go index 991a3d95b903..37c0a86635ea 100644 --- a/models/wiki_test.go +++ b/models/wiki_test.go @@ -161,6 +161,7 @@ func TestRepository_AddWikiPage(t *testing.T) { // Now need to show that the page has been added: gitRepo, err := git.OpenRepository(repo.WikiPath()) assert.NoError(t, err) + defer gitRepo.Close() masterTree, err := gitRepo.GetTree("master") assert.NoError(t, err) wikiPath := WikiNameToFilename(wikiName) @@ -214,6 +215,7 @@ func TestRepository_EditWikiPage(t *testing.T) { _, err := masterTree.GetTreeEntryByPath("Home.md") assert.Error(t, err) } + gitRepo.Close() } } @@ -226,6 +228,7 @@ func TestRepository_DeleteWikiPage(t *testing.T) { // Now need to show that the page has been added: gitRepo, err := git.OpenRepository(repo.WikiPath()) assert.NoError(t, err) + defer gitRepo.Close() masterTree, err := gitRepo.GetTree("master") assert.NoError(t, err) wikiPath := WikiNameToFilename("Home") diff --git a/modules/context/api.go b/modules/context/api.go index 024ae487f115..c1de37dd21a0 100644 --- a/modules/context/api.go +++ b/modules/context/api.go @@ -186,7 +186,16 @@ func ReferencesGitRepo(allowEmpty bool) macaron.Handler { return } ctx.Repo.GitRepo = gitRepo + // We opened it, we should close it + defer func() { + // If it's been set to nil then assume someone else has closed it. + if ctx.Repo.GitRepo != nil { + ctx.Repo.GitRepo.Close() + } + }() } + + ctx.Next() } } diff --git a/modules/context/repo.go b/modules/context/repo.go index 243b98c7c311..1b57b42006e2 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -427,9 +427,18 @@ func RepoAssignment() macaron.Handler { } ctx.Repo.GitRepo = gitRepo + // We opened it, we should close it + defer func() { + // If it's been set to nil then assume someone else has closed it. + if ctx.Repo.GitRepo != nil { + ctx.Repo.GitRepo.Close() + } + }() + // Stop at this point when the repo is empty. if ctx.Repo.Repository.IsEmpty { ctx.Data["BranchName"] = ctx.Repo.Repository.DefaultBranch + ctx.Next() return } @@ -488,6 +497,7 @@ func RepoAssignment() macaron.Handler { ctx.Data["GoDocDirectory"] = prefix + "{/dir}" ctx.Data["GoDocFile"] = prefix + "{/dir}/{file}#L{line}" } + ctx.Next() } } @@ -593,6 +603,13 @@ func RepoRefByType(refType RepoRefType) macaron.Handler { ctx.ServerError("RepoRef Invalid repo "+repoPath, err) return } + // We opened it, we should close it + defer func() { + // If it's been set to nil then assume someone else has closed it. + if ctx.Repo.GitRepo != nil { + ctx.Repo.GitRepo.Close() + } + }() } // Get default branch. @@ -681,6 +698,8 @@ func RepoRefByType(refType RepoRefType) macaron.Handler { return } ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount + + ctx.Next() } } diff --git a/modules/git/blame.go b/modules/git/blame.go index 548236b65702..4f4343fe9642 100644 --- a/modules/git/blame.go +++ b/modules/git/blame.go @@ -87,10 +87,11 @@ func (r *BlameReader) Close() error { // CreateBlameReader creates reader for given repository, commit and file func CreateBlameReader(repoPath, commitID, file string) (*BlameReader, error) { - _, err := OpenRepository(repoPath) + gitRepo, err := OpenRepository(repoPath) if err != nil { return nil, err } + gitRepo.Close() return createBlameReader(repoPath, GitExecutable, "blame", commitID, "--porcelain", "--", file) } diff --git a/modules/git/blob_test.go b/modules/git/blob_test.go index 66c046ecc8d0..9043de5955f4 100644 --- a/modules/git/blob_test.go +++ b/modules/git/blob_test.go @@ -37,6 +37,8 @@ THE SOFTWARE. ` repo, err := OpenRepository("../../.git") assert.NoError(t, err) + defer repo.Close() + testBlob, err := repo.GetBlob("a8d4b49dd073a4a38a7e58385eeff7cc52568697") assert.NoError(t, err) @@ -55,6 +57,8 @@ func Benchmark_Blob_Data(b *testing.B) { if err != nil { b.Fatal(err) } + defer repo.Close() + testBlob, err := repo.GetBlob("a8d4b49dd073a4a38a7e58385eeff7cc52568697") if err != nil { b.Fatal(err) diff --git a/modules/git/commit_info_test.go b/modules/git/commit_info_test.go index 71637d188aaa..ac7bc43c4e21 100644 --- a/modules/git/commit_info_test.go +++ b/modules/git/commit_info_test.go @@ -77,6 +77,8 @@ func TestEntries_GetCommitsInfo(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() + testGetCommitsInfo(t, bareRepo1) clonedPath, err := cloneRepo(bareRepo1Path, testReposDir, "repo1_TestEntries_GetCommitsInfo") @@ -84,6 +86,8 @@ func TestEntries_GetCommitsInfo(t *testing.T) { defer os.RemoveAll(clonedPath) clonedRepo1, err := OpenRepository(clonedPath) assert.NoError(t, err) + defer clonedRepo1.Close() + testGetCommitsInfo(t, clonedRepo1) } @@ -101,13 +105,16 @@ func BenchmarkEntries_GetCommitsInfo(b *testing.B) { for _, benchmark := range benchmarks { var commit *Commit var entries Entries + var repo *Repository if repoPath, err := cloneRepo(benchmark.url, benchmarkReposDir, benchmark.name); err != nil { b.Fatal(err) - } else if repo, err := OpenRepository(repoPath); err != nil { + } else if repo, err = OpenRepository(repoPath); err != nil { b.Fatal(err) } else if commit, err = repo.GetBranchCommit("master"); err != nil { + repo.Close() b.Fatal(err) } else if entries, err = commit.Tree.ListEntries(); err != nil { + repo.Close() b.Fatal(err) } entries.Sort() @@ -120,5 +127,6 @@ func BenchmarkEntries_GetCommitsInfo(b *testing.B) { } } }) + repo.Close() } } diff --git a/modules/git/notes_test.go b/modules/git/notes_test.go index bf010b9a712a..b7939e691355 100644 --- a/modules/git/notes_test.go +++ b/modules/git/notes_test.go @@ -15,6 +15,7 @@ func TestGetNotes(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() note := Note{} err = GetNote(bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653", ¬e) @@ -27,6 +28,7 @@ func TestGetNestedNotes(t *testing.T) { repoPath := filepath.Join(testReposDir, "repo3_notes") repo, err := OpenRepository(repoPath) assert.NoError(t, err) + defer repo.Close() note := Note{} err = GetNote(repo, "3e668dbfac39cbc80a9ff9c61eb565d944453ba4", ¬e) diff --git a/modules/git/repo.go b/modules/git/repo.go index 1a9112132f9f..f3453442c92b 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -17,6 +17,7 @@ import ( "strings" "time" + gitealog "code.gitea.io/gitea/modules/log" "github.com/unknwon/com" "gopkg.in/src-d/go-billy.v4/osfs" gogit "gopkg.in/src-d/go-git.v4" @@ -107,6 +108,21 @@ func OpenRepository(repoPath string) (*Repository, error) { }, nil } +// Close this repository, in particular close the underlying gogitStorage if this is not nil +func (repo *Repository) Close() { + if repo == nil || repo.gogitStorage == nil { + return + } + if err := repo.gogitStorage.Close(); err != nil { + gitealog.Error("Error closing storage: %v", err) + } +} + +// GoGitRepo gets the go-git repo representation +func (repo *Repository) GoGitRepo() *gogit.Repository { + return repo.gogitRepo +} + // IsEmpty Check if repository is empty. func (repo *Repository) IsEmpty() (bool, error) { var errbuf strings.Builder diff --git a/modules/git/repo_blob_test.go b/modules/git/repo_blob_test.go index 128a227829c1..52a124db2a75 100644 --- a/modules/git/repo_blob_test.go +++ b/modules/git/repo_blob_test.go @@ -17,6 +17,7 @@ func TestRepository_GetBlob_Found(t *testing.T) { repoPath := filepath.Join(testReposDir, "repo1_bare") r, err := OpenRepository(repoPath) assert.NoError(t, err) + defer r.Close() testCases := []struct { OID string @@ -44,6 +45,7 @@ func TestRepository_GetBlob_NotExist(t *testing.T) { repoPath := filepath.Join(testReposDir, "repo1_bare") r, err := OpenRepository(repoPath) assert.NoError(t, err) + defer r.Close() testCase := "0000000000000000000000000000000000000000" testError := ErrNotExist{testCase, ""} @@ -57,6 +59,7 @@ func TestRepository_GetBlob_NoId(t *testing.T) { repoPath := filepath.Join(testReposDir, "repo1_bare") r, err := OpenRepository(repoPath) assert.NoError(t, err) + defer r.Close() testCase := "" testError := fmt.Errorf("Length must be 40: %s", testCase) diff --git a/modules/git/repo_branch.go b/modules/git/repo_branch.go index a2bf9ac973e4..e79bab76a6f3 100644 --- a/modules/git/repo_branch.go +++ b/modules/git/repo_branch.go @@ -108,6 +108,7 @@ func GetBranchesByPath(path string) ([]*Branch, error) { if err != nil { return nil, err } + defer gitRepo.Close() brs, err := gitRepo.GetBranches() if err != nil { diff --git a/modules/git/repo_branch_test.go b/modules/git/repo_branch_test.go index 08736d702e21..33d31aef686d 100644 --- a/modules/git/repo_branch_test.go +++ b/modules/git/repo_branch_test.go @@ -15,6 +15,7 @@ func TestRepository_GetBranches(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() branches, err := bareRepo1.GetBranches() @@ -29,6 +30,7 @@ func BenchmarkRepository_GetBranches(b *testing.B) { if err != nil { b.Fatal(err) } + defer bareRepo1.Close() for i := 0; i < b.N; i++ { _, err := bareRepo1.GetBranches() diff --git a/modules/git/repo_commit_test.go b/modules/git/repo_commit_test.go index 6d8ee6453f5a..87dd6763b394 100644 --- a/modules/git/repo_commit_test.go +++ b/modules/git/repo_commit_test.go @@ -15,6 +15,7 @@ func TestRepository_GetCommitBranches(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() // these test case are specific to the repo1_bare test repo testCases := []struct { @@ -41,6 +42,7 @@ func TestGetTagCommitWithSignature(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() commit, err := bareRepo1.GetCommit("3ad28a9149a2864384548f3d17ed7f38014c9e8a") assert.NoError(t, err) @@ -54,6 +56,7 @@ func TestGetCommitWithBadCommitID(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() commit, err := bareRepo1.GetCommit("bad_branch") assert.Nil(t, commit) diff --git a/modules/git/repo_compare_test.go b/modules/git/repo_compare_test.go index e19478877341..def67fa87b7f 100644 --- a/modules/git/repo_compare_test.go +++ b/modules/git/repo_compare_test.go @@ -20,6 +20,7 @@ func TestGetFormatPatch(t *testing.T) { defer os.RemoveAll(clonedPath) repo, err := OpenRepository(clonedPath) assert.NoError(t, err) + defer repo.Close() rd, err := repo.GetFormatPatch("8d92fc95^", "8d92fc95") assert.NoError(t, err) patchb, err := ioutil.ReadAll(rd) diff --git a/modules/git/repo_ref_test.go b/modules/git/repo_ref_test.go index d32b34994c39..303c496c1ddf 100644 --- a/modules/git/repo_ref_test.go +++ b/modules/git/repo_ref_test.go @@ -15,6 +15,7 @@ func TestRepository_GetRefs(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() refs, err := bareRepo1.GetRefs() @@ -38,6 +39,7 @@ func TestRepository_GetRefsFiltered(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() refs, err := bareRepo1.GetRefsFiltered(TagPrefix) diff --git a/modules/git/repo_stats_test.go b/modules/git/repo_stats_test.go index 6fbcb7ac13c0..bc1f6a566262 100644 --- a/modules/git/repo_stats_test.go +++ b/modules/git/repo_stats_test.go @@ -16,6 +16,7 @@ func TestRepository_GetCodeActivityStats(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() timeFrom, err := time.Parse(time.RFC3339, "2016-01-01T00:00:00+00:00") assert.NoError(t, err) diff --git a/modules/git/repo_tag_test.go b/modules/git/repo_tag_test.go index ab9742afc596..90f2b37358d7 100644 --- a/modules/git/repo_tag_test.go +++ b/modules/git/repo_tag_test.go @@ -16,6 +16,7 @@ func TestRepository_GetTags(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") bareRepo1, err := OpenRepository(bareRepo1Path) assert.NoError(t, err) + defer bareRepo1.Close() tags, err := bareRepo1.GetTagInfos() assert.NoError(t, err) @@ -34,6 +35,7 @@ func TestRepository_GetTag(t *testing.T) { bareRepo1, err := OpenRepository(clonedPath) assert.NoError(t, err) + defer bareRepo1.Close() lTagCommitID := "6fbd69e9823458e6c4a2fc5c0f6bc022b2f2acd1" lTagName := "lightweightTag" @@ -83,6 +85,7 @@ func TestRepository_GetAnnotatedTag(t *testing.T) { bareRepo1, err := OpenRepository(clonedPath) assert.NoError(t, err) + defer bareRepo1.Close() lTagCommitID := "6fbd69e9823458e6c4a2fc5c0f6bc022b2f2acd1" lTagName := "lightweightTag" diff --git a/modules/git/repo_test.go b/modules/git/repo_test.go index 15f5e3781c05..0b6986764c05 100644 --- a/modules/git/repo_test.go +++ b/modules/git/repo_test.go @@ -30,6 +30,7 @@ func TestRepoIsEmpty(t *testing.T) { emptyRepo2Path := filepath.Join(testReposDir, "repo2_empty") repo, err := OpenRepository(emptyRepo2Path) assert.NoError(t, err) + defer repo.Close() isEmpty, err := repo.IsEmpty() assert.NoError(t, err) assert.True(t, isEmpty) diff --git a/modules/git/tree_entry_test.go b/modules/git/tree_entry_test.go index c65a691ecf20..e8729003703e 100644 --- a/modules/git/tree_entry_test.go +++ b/modules/git/tree_entry_test.go @@ -56,6 +56,7 @@ func TestEntriesCustomSort(t *testing.T) { func TestFollowLink(t *testing.T) { r, err := OpenRepository("tests/repos/repo1_bare") assert.NoError(t, err) + defer r.Close() commit, err := r.GetCommit("37991dec2c8e592043f47155ce4808d4580f9123") assert.NoError(t, err) diff --git a/modules/migrations/base/uploader.go b/modules/migrations/base/uploader.go index a3a9c9fac619..ae1be84b88a9 100644 --- a/modules/migrations/base/uploader.go +++ b/modules/migrations/base/uploader.go @@ -17,4 +17,5 @@ type Uploader interface { CreateComments(comments ...*Comment) error CreatePullRequests(prs ...*PullRequest) error Rollback() error + Close() } diff --git a/modules/migrations/gitea.go b/modules/migrations/gitea.go index 676667b426aa..81a6116a237d 100644 --- a/modules/migrations/gitea.go +++ b/modules/migrations/gitea.go @@ -131,6 +131,13 @@ func (g *GiteaLocalUploader) CreateRepo(repo *base.Repository, opts base.Migrate return err } +// Close closes this uploader +func (g *GiteaLocalUploader) Close() { + if g.gitRepo != nil { + g.gitRepo.Close() + } +} + // CreateTopics creates topics func (g *GiteaLocalUploader) CreateTopics(topics ...string) error { return models.SaveTopics(g.repo.ID, topics...) diff --git a/modules/migrations/migrate.go b/modules/migrations/migrate.go index bbc1dc2d5610..7a5071e1258d 100644 --- a/modules/migrations/migrate.go +++ b/modules/migrations/migrate.go @@ -94,6 +94,7 @@ func migrateRepository(downloader base.Downloader, uploader base.Uploader, opts if err := uploader.CreateRepo(repo, opts); err != nil { return err } + defer uploader.Close() log.Trace("migrating topics") topics, err := downloader.GetTopics() diff --git a/modules/repofiles/action.go b/modules/repofiles/action.go index 9467e4fb72f5..755c015ca9d0 100644 --- a/modules/repofiles/action.go +++ b/modules/repofiles/action.go @@ -53,9 +53,11 @@ func CommitRepoAction(opts CommitRepoActionOptions) error { } if err := gitRepo.SetDefaultBranch(repo.DefaultBranch); err != nil { if !git.IsErrUnsupportedVersion(err) { + gitRepo.Close() return err } } + gitRepo.Close() } } @@ -132,8 +134,10 @@ func CommitRepoAction(opts CommitRepoActionOptions) error { shaSum, err = gitRepo.GetBranchCommitID(refName) if err != nil { + gitRepo.Close() log.Error("GetBranchCommitID[%s]: %v", opts.RefFullName, err) } + gitRepo.Close() if err = models.PrepareWebhooks(repo, models.HookEventCreate, &api.CreatePayload{ Ref: refName, Sha: shaSum, @@ -167,8 +171,10 @@ func CommitRepoAction(opts CommitRepoActionOptions) error { } shaSum, err = gitRepo.GetTagCommitID(refName) if err != nil { + gitRepo.Close() log.Error("GetTagCommitID[%s]: %v", opts.RefFullName, err) } + gitRepo.Close() if err = models.PrepareWebhooks(repo, models.HookEventCreate, &api.CreatePayload{ Ref: refName, Sha: shaSum, diff --git a/modules/repofiles/blob.go b/modules/repofiles/blob.go index e9d85a0dcf25..60a05e280e86 100644 --- a/modules/repofiles/blob.go +++ b/modules/repofiles/blob.go @@ -17,6 +17,7 @@ func GetBlobBySHA(repo *models.Repository, sha string) (*api.GitBlobResponse, er if err != nil { return nil, err } + defer gitRepo.Close() gitBlob, err := gitRepo.GetBlob(sha) if err != nil { return nil, err diff --git a/modules/repofiles/blob_test.go b/modules/repofiles/blob_test.go index 1dc183a8afb3..ddc23aeac351 100644 --- a/modules/repofiles/blob_test.go +++ b/modules/repofiles/blob_test.go @@ -21,6 +21,8 @@ func TestGetBlobBySHA(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + sha := "65f1bf27bc3bf70f64657658635e66094edbcb4d" ctx.SetParams(":id", "1") ctx.SetParams(":sha", sha) diff --git a/modules/repofiles/commit_status.go b/modules/repofiles/commit_status.go index f3dfbf209f7d..3d93c58d8582 100644 --- a/modules/repofiles/commit_status.go +++ b/modules/repofiles/commit_status.go @@ -23,8 +23,10 @@ func CreateCommitStatus(repo *models.Repository, creator *models.User, sha strin return fmt.Errorf("OpenRepository[%s]: %v", repoPath, err) } if _, err := gitRepo.GetCommit(sha); err != nil { + gitRepo.Close() return fmt.Errorf("GetCommit[%s]: %v", sha, err) } + gitRepo.Close() if err := models.NewCommitStatus(models.NewCommitStatusOptions{ Repo: repo, diff --git a/modules/repofiles/content.go b/modules/repofiles/content.go index d7d43ef9d166..aed98c33a8c4 100644 --- a/modules/repofiles/content.go +++ b/modules/repofiles/content.go @@ -59,6 +59,7 @@ func GetContentsOrList(repo *models.Repository, treePath, ref string) (interface if err != nil { return nil, err } + defer gitRepo.Close() // Get the commit object for the ref commit, err := gitRepo.GetCommit(ref) @@ -117,6 +118,7 @@ func GetContents(repo *models.Repository, treePath, ref string, forList bool) (* if err != nil { return nil, err } + defer gitRepo.Close() // Get the commit object for the ref commit, err := gitRepo.GetCommit(ref) diff --git a/modules/repofiles/content_test.go b/modules/repofiles/content_test.go index cd98c54ea69f..d024cfd54982 100644 --- a/modules/repofiles/content_test.go +++ b/modules/repofiles/content_test.go @@ -56,6 +56,8 @@ func TestGetContents(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + treePath := "README.md" ref := ctx.Repo.Repository.DefaultBranch @@ -82,6 +84,8 @@ func TestGetContentsOrListForDir(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + treePath := "" // root dir ref := ctx.Repo.Repository.DefaultBranch @@ -115,6 +119,8 @@ func TestGetContentsOrListForFile(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + treePath := "README.md" ref := ctx.Repo.Repository.DefaultBranch @@ -141,6 +147,8 @@ func TestGetContentsErrors(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository treePath := "README.md" ref := repo.DefaultBranch @@ -170,6 +178,8 @@ func TestGetContentsOrListErrors(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository treePath := "README.md" ref := repo.DefaultBranch @@ -198,6 +208,8 @@ func TestGetContentsOrListOfEmptyRepos(t *testing.T) { test.LoadRepo(t, ctx, 15) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository t.Run("empty repo", func(t *testing.T) { diff --git a/modules/repofiles/diff_test.go b/modules/repofiles/diff_test.go index de5ed1d75485..db2c7552c4ff 100644 --- a/modules/repofiles/diff_test.go +++ b/modules/repofiles/diff_test.go @@ -22,6 +22,8 @@ func TestGetDiffPreview(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + branch := ctx.Repo.Repository.DefaultBranch treePath := "README.md" content := "# repo1\n\nDescription for repo1\nthis is a new line" @@ -119,6 +121,8 @@ func TestGetDiffPreviewErrors(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + branch := ctx.Repo.Repository.DefaultBranch treePath := "README.md" content := "# repo1\n\nDescription for repo1\nthis is a new line" diff --git a/modules/repofiles/file_test.go b/modules/repofiles/file_test.go index 7c45139dd94f..46b5a6c54961 100644 --- a/modules/repofiles/file_test.go +++ b/modules/repofiles/file_test.go @@ -88,10 +88,13 @@ func TestGetFileResponseFromCommit(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository branch := repo.DefaultBranch treePath := "README.md" gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() commit, _ := gitRepo.GetBranchCommit(branch) expectedFileResponse := getExpectedFileResponse() diff --git a/modules/repofiles/temp_repo.go b/modules/repofiles/temp_repo.go index 4a50e641927d..ae4d90594612 100644 --- a/modules/repofiles/temp_repo.go +++ b/modules/repofiles/temp_repo.go @@ -44,6 +44,7 @@ func NewTemporaryUploadRepository(repo *models.Repository) (*TemporaryUploadRepo // Close the repository cleaning up all files func (t *TemporaryUploadRepository) Close() { + defer t.gitRepo.Close() if err := models.RemoveTemporaryPath(t.basePath); err != nil { log.Error("Failed to remove temporary path %s: %v", t.basePath, err) } diff --git a/modules/repofiles/tree.go b/modules/repofiles/tree.go index 318a5d152d83..cf0534563fc3 100644 --- a/modules/repofiles/tree.go +++ b/modules/repofiles/tree.go @@ -19,6 +19,7 @@ func GetTreeBySHA(repo *models.Repository, sha string, page, perPage int, recurs if err != nil { return nil, err } + defer gitRepo.Close() gitTree, err := gitRepo.GetTree(sha) if err != nil || gitTree == nil { return nil, models.ErrSHANotFound{ diff --git a/modules/repofiles/tree_test.go b/modules/repofiles/tree_test.go index ecff8b90717e..e1bb812ec156 100644 --- a/modules/repofiles/tree_test.go +++ b/modules/repofiles/tree_test.go @@ -21,6 +21,8 @@ func TestGetTreeBySHA(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + sha := ctx.Repo.Repository.DefaultBranch page := 1 perPage := 10 diff --git a/modules/repofiles/update.go b/modules/repofiles/update.go index 1a1fe6c389f4..19dcc87e296b 100644 --- a/modules/repofiles/update.go +++ b/modules/repofiles/update.go @@ -429,6 +429,7 @@ func PushUpdate(repo *models.Repository, branch string, opts models.PushUpdateOp if err != nil { return fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() if err = repo.UpdateSize(); err != nil { log.Error("Failed to update size for repository: %v", err) diff --git a/modules/test/context_tests.go b/modules/test/context_tests.go index 92df1c576252..cf9c5fbc548a 100644 --- a/modules/test/context_tests.go +++ b/modules/test/context_tests.go @@ -55,6 +55,7 @@ func LoadRepo(t *testing.T, ctx *context.Context, repoID int64) { func LoadRepoCommit(t *testing.T, ctx *context.Context) { gitRepo, err := git.OpenRepository(ctx.Repo.Repository.RepoPath()) assert.NoError(t, err) + defer gitRepo.Close() branch, err := gitRepo.GetHEADBranch() assert.NoError(t, err) ctx.Repo.Commit, err = gitRepo.GetBranchCommit(branch.Name) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 04ff91fbbf93..10b827253db9 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -638,7 +638,7 @@ func RegisterRoutes(m *macaron.Macaron) { }, reqRepoReader(models.UnitTypeCode)) m.Group("/tags", func() { m.Get("", repo.ListTags) - }, reqRepoReader(models.UnitTypeCode)) + }, reqRepoReader(models.UnitTypeCode), context.ReferencesGitRepo(true)) m.Group("/keys", func() { m.Combo("").Get(repo.ListDeployKeys). Post(bind(api.CreateKeyOption{}), repo.CreateDeployKey) diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index 0156aaaa05cd..163a06a95e8e 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -51,6 +51,7 @@ func GetSingleCommit(ctx *context.APIContext) { ctx.ServerError("OpenRepository", err) return } + defer gitRepo.Close() commit, err := gitRepo.GetCommit(ctx.Params(":sha")) if err != nil { ctx.NotFoundOrServerError("GetCommit", git.IsErrNotExist, err) @@ -113,6 +114,7 @@ func GetAllCommits(ctx *context.APIContext) { ctx.ServerError("OpenRepository", err) return } + defer gitRepo.Close() page := ctx.QueryInt("page") if page <= 0 { diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index ae20e1e96be9..f23e7bca9ea1 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -95,6 +95,7 @@ func GetArchive(ctx *context.APIContext) { return } ctx.Repo.GitRepo = gitRepo + defer gitRepo.Close() repo.Download(ctx.Context) } diff --git a/routers/api/v1/repo/git_ref.go b/routers/api/v1/repo/git_ref.go index d7acc139f0d5..c2bcbb360386 100644 --- a/routers/api/v1/repo/git_ref.go +++ b/routers/api/v1/repo/git_ref.go @@ -76,6 +76,8 @@ func getGitRefs(ctx *context.APIContext, filter string) ([]*git.Reference, strin if err != nil { return nil, "OpenRepository", err } + defer gitRepo.Close() + if len(filter) > 0 { filter = "refs/" + filter } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 4d6a84d8f8f4..2601ab8339e4 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -194,6 +194,7 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption if ctx.Written() { return } + defer headGitRepo.Close() // Check if another PR exists with the same targets existingPr, err := models.GetUnmergedPullRequest(headRepo.ID, ctx.Repo.Repository.ID, headBranch, baseBranch) @@ -687,6 +688,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) // user should have permission to read baseRepo's codes and pulls, NOT headRepo's permBase, err := models.GetUserRepoPermission(baseRepo, ctx.User) if err != nil { + headGitRepo.Close() ctx.ServerError("GetUserRepoPermission", err) return nil, nil, nil, nil, "", "" } @@ -697,6 +699,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) baseRepo, permBase) } + headGitRepo.Close() ctx.NotFound("Can't read pulls or can't read UnitTypeCode") return nil, nil, nil, nil, "", "" } @@ -704,6 +707,7 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) // user should have permission to read headrepo's codes permHead, err := models.GetUserRepoPermission(headRepo, ctx.User) if err != nil { + headGitRepo.Close() ctx.ServerError("GetUserRepoPermission", err) return nil, nil, nil, nil, "", "" } @@ -714,18 +718,21 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) headRepo, permHead) } + headGitRepo.Close() ctx.NotFound("Can't read headRepo UnitTypeCode") return nil, nil, nil, nil, "", "" } // Check if head branch is valid. if !headGitRepo.IsBranchExist(headBranch) { + headGitRepo.Close() ctx.NotFound() return nil, nil, nil, nil, "", "" } compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranch, headBranch) if err != nil { + headGitRepo.Close() ctx.Error(500, "GetCompareInfo", err) return nil, nil, nil, nil, "", "" } diff --git a/routers/api/v1/repo/tag.go b/routers/api/v1/repo/tag.go index a802048285d6..12bac854ec0b 100644 --- a/routers/api/v1/repo/tag.go +++ b/routers/api/v1/repo/tag.go @@ -33,7 +33,7 @@ func ListTags(ctx *context.APIContext) { // responses: // "200": // "$ref": "#/responses/TagList" - tags, err := ctx.Repo.Repository.GetTags() + tags, err := ctx.Repo.GitRepo.GetTagInfos() if err != nil { ctx.Error(500, "GetTags", err) return diff --git a/routers/repo/compare.go b/routers/repo/compare.go index f8534f68b77b..2b66d3818d6f 100644 --- a/routers/repo/compare.go +++ b/routers/repo/compare.go @@ -157,6 +157,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * // user should have permission to read baseRepo's codes and pulls, NOT headRepo's permBase, err := models.GetUserRepoPermission(baseRepo, ctx.User) if err != nil { + headGitRepo.Close() ctx.ServerError("GetUserRepoPermission", err) return nil, nil, nil, nil, "", "" } @@ -167,6 +168,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * baseRepo, permBase) } + headGitRepo.Close() ctx.NotFound("ParseCompareInfo", nil) return nil, nil, nil, nil, "", "" } @@ -174,6 +176,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * // user should have permission to read headrepo's codes permHead, err := models.GetUserRepoPermission(headRepo, ctx.User) if err != nil { + headGitRepo.Close() ctx.ServerError("GetUserRepoPermission", err) return nil, nil, nil, nil, "", "" } @@ -184,6 +187,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * headRepo, permHead) } + headGitRepo.Close() ctx.NotFound("ParseCompareInfo", nil) return nil, nil, nil, nil, "", "" } @@ -199,6 +203,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * ctx.Data["HeadBranch"] = headBranch headIsCommit = true } else { + headGitRepo.Close() ctx.NotFound("IsRefExist", nil) return nil, nil, nil, nil, "", "" } @@ -219,12 +224,14 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, * baseRepo, permBase) } + headGitRepo.Close() ctx.NotFound("ParseCompareInfo", nil) return nil, nil, nil, nil, "", "" } compareInfo, err := headGitRepo.GetCompareInfo(models.RepoPath(baseRepo.Owner.Name, baseRepo.Name), baseBranch, headBranch) if err != nil { + headGitRepo.Close() ctx.ServerError("GetCompareInfo", err) return nil, nil, nil, nil, "", "" } @@ -345,6 +352,7 @@ func CompareDiff(ctx *context.Context) { if ctx.Written() { return } + defer headGitRepo.Close() nothingToCompare := PrepareCompareDiff(ctx, headUser, headRepo, headGitRepo, compareInfo, baseBranch, headBranch) if ctx.Written() { diff --git a/routers/repo/editor_test.go b/routers/repo/editor_test.go index ca00be74b7fe..ec7aee1e77ff 100644 --- a/routers/repo/editor_test.go +++ b/routers/repo/editor_test.go @@ -48,6 +48,8 @@ func TestGetUniquePatchBranchName(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + expectedBranchName := "user2-patch-1" branchName := GetUniquePatchBranchName(ctx) assert.Equal(t, expectedBranchName, branchName) @@ -61,9 +63,12 @@ func TestGetClosestParentWithFiles(t *testing.T) { test.LoadRepoCommit(t, ctx) test.LoadUser(t, ctx, 2) test.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + repo := ctx.Repo.Repository branch := repo.DefaultBranch gitRepo, _ := git.OpenRepository(repo.RepoPath()) + defer gitRepo.Close() commit, _ := gitRepo.GetBranchCommit(branch) expectedTreePath := "" diff --git a/routers/repo/pull.go b/routers/repo/pull.go index 23d1718cf295..33bf1b2ed979 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -337,6 +337,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare ctx.ServerError("OpenRepository", err) return nil } + defer headGitRepo.Close() headBranchExist = headGitRepo.IsBranchExist(pull.HeadBranch) @@ -519,6 +520,7 @@ func ViewPullFiles(ctx *context.Context) { ctx.ServerError("OpenRepository", err) return } + defer headGitRepo.Close() headCommitID, err := headGitRepo.GetBranchCommitID(pull.HeadBranch) if err != nil { @@ -704,6 +706,7 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm) if ctx.Written() { return } + defer headGitRepo.Close() labelIDs, assigneeIDs, milestoneID := ValidateRepoMetas(ctx, form, true) if ctx.Written() { @@ -870,12 +873,14 @@ func CleanUpPullRequest(ctx *context.Context) { ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err) return } + defer gitRepo.Close() gitBaseRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) if err != nil { ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.BaseRepo.RepoPath()), err) return } + defer gitBaseRepo.Close() defer func() { ctx.JSON(200, map[string]interface{}{ @@ -1004,6 +1009,7 @@ func DownloadPullPatch(ctx *context.Context) { ctx.ServerError("OpenRepository", err) return } + defer headGitRepo.Close() patch, err := headGitRepo.GetFormatPatch(pr.MergeBase, pr.HeadBranch) if err != nil { diff --git a/routers/repo/release_test.go b/routers/repo/release_test.go index 524c1c7346bc..47d1a89b54a0 100644 --- a/routers/repo/release_test.go +++ b/routers/repo/release_test.go @@ -57,5 +57,6 @@ func TestNewReleasePost(t *testing.T) { Title: testCase.Form.Title, Note: testCase.Form.Content, }, models.Cond("is_draft=?", len(testCase.Form.Draft) > 0)) + ctx.Repo.GitRepo.Close() } } diff --git a/routers/repo/setting.go b/routers/repo/setting.go index c74b273420de..eb4f27e64bee 100644 --- a/routers/repo/setting.go +++ b/routers/repo/setting.go @@ -71,6 +71,11 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { // Check if repository name has been changed. if repo.LowerName != strings.ToLower(newRepoName) { isNameChanged = true + // Close the GitRepo if open + if ctx.Repo.GitRepo != nil { + ctx.Repo.GitRepo.Close() + ctx.Repo.GitRepo = nil + } if err := models.ChangeRepositoryName(ctx.Repo.Owner, repo.Name, newRepoName); err != nil { ctx.Data["Err_RepoName"] = true switch { @@ -378,6 +383,11 @@ func SettingsPost(ctx *context.Context, form auth.RepoSettingForm) { } oldOwnerID := ctx.Repo.Owner.ID + // Close the GitRepo if open + if ctx.Repo.GitRepo != nil { + ctx.Repo.GitRepo.Close() + ctx.Repo.GitRepo = nil + } if err = models.TransferOwnership(ctx.User, newOwner, repo); err != nil { if models.IsErrRepoAlreadyExist(err) { ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplSettingsOptions, nil) diff --git a/routers/repo/wiki.go b/routers/repo/wiki.go index 02fbe4a1ddad..6cf194365891 100644 --- a/routers/repo/wiki.go +++ b/routers/repo/wiki.go @@ -146,6 +146,9 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { // Get page list. entries, err := commit.ListEntries() if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } ctx.ServerError("ListEntries", err) return nil, nil } @@ -159,6 +162,9 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { if models.IsErrWikiInvalidFileName(err) { continue } + if wikiRepo != nil { + wikiRepo.Close() + } ctx.ServerError("WikiFilenameToName", err) return nil, nil } else if wikiName == "_Sidebar" || wikiName == "_Footer" { @@ -188,16 +194,25 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { ctx.Redirect(ctx.Repo.RepoLink + "/wiki/_pages") } if entry == nil || ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return nil, nil } sidebarContent, _, _, _ := wikiContentsByName(ctx, commit, "_Sidebar") if ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return nil, nil } footerContent, _, _, _ := wikiContentsByName(ctx, commit, "_Footer") if ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return nil, nil } @@ -218,6 +233,9 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { wikiRepo, commit, err := findWikiRepoCommit(ctx) if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } if !git.IsErrNotExist(err) { ctx.ServerError("GetBranchCommit", err) } @@ -241,6 +259,9 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) ctx.Redirect(ctx.Repo.RepoLink + "/wiki/_pages") } if entry == nil || ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return nil, nil } @@ -263,6 +284,9 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) // get Commit Count commitsHistory, err := wikiRepo.CommitsByFileAndRangeNoFollow("master", pageFilename, page) if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } ctx.ServerError("CommitsByFileAndRangeNoFollow", err) return nil, nil } @@ -279,13 +303,21 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) } func renderEditPage(ctx *context.Context) { - _, commit, err := findWikiRepoCommit(ctx) + wikiRepo, commit, err := findWikiRepoCommit(ctx) if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } if !git.IsErrNotExist(err) { ctx.ServerError("GetBranchCommit", err) } return } + defer func() { + if wikiRepo != nil { + wikiRepo.Close() + } + }() // get requested pagename pageName := models.NormalizeWikiName(ctx.Params(":page")) @@ -327,8 +359,16 @@ func Wiki(ctx *context.Context) { wikiRepo, entry := renderViewPage(ctx) if ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return } + defer func() { + if wikiRepo != nil { + wikiRepo.Close() + } + }() if entry == nil { ctx.Data["Title"] = ctx.Tr("repo.wiki") ctx.HTML(200, tplWikiStart) @@ -364,8 +404,16 @@ func WikiRevision(ctx *context.Context) { wikiRepo, entry := renderRevisionPage(ctx) if ctx.Written() { + if wikiRepo != nil { + wikiRepo.Close() + } return } + defer func() { + if wikiRepo != nil { + wikiRepo.Close() + } + }() if entry == nil { ctx.Data["Title"] = ctx.Tr("repo.wiki") ctx.HTML(200, tplWikiStart) @@ -397,11 +445,18 @@ func WikiPages(ctx *context.Context) { wikiRepo, commit, err := findWikiRepoCommit(ctx) if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } return } entries, err := commit.ListEntries() if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } + ctx.ServerError("ListEntries", err) return } @@ -412,6 +467,10 @@ func WikiPages(ctx *context.Context) { } c, err := wikiRepo.GetCommitByPath(entry.Name()) if err != nil { + if wikiRepo != nil { + wikiRepo.Close() + } + ctx.ServerError("GetCommit", err) return } @@ -420,6 +479,10 @@ func WikiPages(ctx *context.Context) { if models.IsErrWikiInvalidFileName(err) { continue } + if wikiRepo != nil { + wikiRepo.Close() + } + ctx.ServerError("WikiFilenameToName", err) return } @@ -431,6 +494,11 @@ func WikiPages(ctx *context.Context) { } ctx.Data["Pages"] = pages + defer func() { + if wikiRepo != nil { + wikiRepo.Close() + } + }() ctx.HTML(200, tplWikiPages) } diff --git a/routers/repo/wiki_test.go b/routers/repo/wiki_test.go index 4687d24f6b28..44fcd02035fe 100644 --- a/routers/repo/wiki_test.go +++ b/routers/repo/wiki_test.go @@ -23,6 +23,7 @@ const message = "Wiki commit message for unit tests" func wikiEntry(t *testing.T, repo *models.Repository, wikiName string) *git.TreeEntry { wikiRepo, err := git.OpenRepository(repo.WikiPath()) assert.NoError(t, err) + defer wikiRepo.Close() commit, err := wikiRepo.GetBranchCommit("master") assert.NoError(t, err) entries, err := commit.ListEntries() diff --git a/services/comments/comments.go b/services/comments/comments.go index e8448e9065ef..406cd79ce6f3 100644 --- a/services/comments/comments.go +++ b/services/comments/comments.go @@ -60,6 +60,7 @@ func CreateCodeComment(doer *models.User, repo *models.Repository, issue *models if err != nil { return nil, fmt.Errorf("OpenRepository: %v", err) } + defer gitRepo.Close() // FIXME validate treePath // Get latest commit referencing the commented line diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index c2c5675d9fd2..9c2aef5c97ce 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -678,6 +678,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID if err != nil { return nil, err } + defer gitRepo.Close() commit, err := gitRepo.GetCommit(afterCommitID) if err != nil { @@ -750,6 +751,7 @@ func GetRawDiffForFile(repoPath, startCommit, endCommit string, diffType RawDiff if err != nil { return fmt.Errorf("OpenRepository: %v", err) } + defer repo.Close() commit, err := repo.GetCommit(endCommit) if err != nil { diff --git a/services/mirror/mirror.go b/services/mirror/mirror.go index 7bfc5fd4da04..a5e956f3d36c 100644 --- a/services/mirror/mirror.go +++ b/services/mirror/mirror.go @@ -197,8 +197,10 @@ func runSync(m *models.Mirror) ([]*mirrorSyncResult, bool) { return nil, false } if err = models.SyncReleasesWithTags(m.Repo, gitRepo); err != nil { + gitRepo.Close() log.Error("Failed to synchronize tags to releases for repository: %v", err) } + gitRepo.Close() if err := m.Repo.UpdateSize(); err != nil { log.Error("Failed to update size for mirror repository: %v", err) @@ -290,97 +292,103 @@ func Update() { func SyncMirrors() { // Start listening on new sync requests. for repoID := range mirrorQueue.Queue() { - log.Trace("SyncMirrors [repo_id: %v]", repoID) - mirrorQueue.Remove(repoID) + syncMirror(repoID) + } +} + +func syncMirror(repoID string) { + log.Trace("SyncMirrors [repo_id: %v]", repoID) + mirrorQueue.Remove(repoID) + + m, err := models.GetMirrorByRepoID(com.StrTo(repoID).MustInt64()) + if err != nil { + log.Error("GetMirrorByRepoID [%s]: %v", repoID, err) + return + + } + + results, ok := runSync(m) + if !ok { + return + } + + m.ScheduleNextUpdate() + if err = models.UpdateMirror(m); err != nil { + log.Error("UpdateMirror [%s]: %v", repoID, err) + return + } - m, err := models.GetMirrorByRepoID(com.StrTo(repoID).MustInt64()) + var gitRepo *git.Repository + if len(results) == 0 { + log.Trace("SyncMirrors [repo_id: %d]: no commits fetched", m.RepoID) + } else { + gitRepo, err = git.OpenRepository(m.Repo.RepoPath()) if err != nil { - log.Error("GetMirrorByRepoID [%s]: %v", repoID, err) - continue + log.Error("OpenRepository [%d]: %v", m.RepoID, err) + return } + defer gitRepo.Close() + } - results, ok := runSync(m) - if !ok { + for _, result := range results { + // Discard GitHub pull requests, i.e. refs/pull/* + if strings.HasPrefix(result.refName, "refs/pull/") { continue } - m.ScheduleNextUpdate() - if err = models.UpdateMirror(m); err != nil { - log.Error("UpdateMirror [%s]: %v", repoID, err) + // Create reference + if result.oldCommitID == gitShortEmptySha { + if err = models.MirrorSyncCreateAction(m.Repo, result.refName); err != nil { + log.Error("MirrorSyncCreateAction [repo_id: %d]: %v", m.RepoID, err) + } continue } - var gitRepo *git.Repository - if len(results) == 0 { - log.Trace("SyncMirrors [repo_id: %d]: no commits fetched", m.RepoID) - } else { - gitRepo, err = git.OpenRepository(m.Repo.RepoPath()) - if err != nil { - log.Error("OpenRepository [%d]: %v", m.RepoID, err) - continue + // Delete reference + if result.newCommitID == gitShortEmptySha { + if err = models.MirrorSyncDeleteAction(m.Repo, result.refName); err != nil { + log.Error("MirrorSyncDeleteAction [repo_id: %d]: %v", m.RepoID, err) } + continue } - for _, result := range results { - // Discard GitHub pull requests, i.e. refs/pull/* - if strings.HasPrefix(result.refName, "refs/pull/") { - continue - } - - // Create reference - if result.oldCommitID == gitShortEmptySha { - if err = models.MirrorSyncCreateAction(m.Repo, result.refName); err != nil { - log.Error("MirrorSyncCreateAction [repo_id: %d]: %v", m.RepoID, err) - } - continue - } - - // Delete reference - if result.newCommitID == gitShortEmptySha { - if err = models.MirrorSyncDeleteAction(m.Repo, result.refName); err != nil { - log.Error("MirrorSyncDeleteAction [repo_id: %d]: %v", m.RepoID, err) - } - continue - } - - // Push commits - oldCommitID, err := git.GetFullCommitID(gitRepo.Path, result.oldCommitID) - if err != nil { - log.Error("GetFullCommitID [%d]: %v", m.RepoID, err) - continue - } - newCommitID, err := git.GetFullCommitID(gitRepo.Path, result.newCommitID) - if err != nil { - log.Error("GetFullCommitID [%d]: %v", m.RepoID, err) - continue - } - commits, err := gitRepo.CommitsBetweenIDs(newCommitID, oldCommitID) - if err != nil { - log.Error("CommitsBetweenIDs [repo_id: %d, new_commit_id: %s, old_commit_id: %s]: %v", m.RepoID, newCommitID, oldCommitID, err) - continue - } - if err = models.MirrorSyncPushAction(m.Repo, models.MirrorSyncPushActionOptions{ - RefName: result.refName, - OldCommitID: oldCommitID, - NewCommitID: newCommitID, - Commits: models.ListToPushCommits(commits), - }); err != nil { - log.Error("MirrorSyncPushAction [repo_id: %d]: %v", m.RepoID, err) - continue - } + // Push commits + oldCommitID, err := git.GetFullCommitID(gitRepo.Path, result.oldCommitID) + if err != nil { + log.Error("GetFullCommitID [%d]: %v", m.RepoID, err) + continue } - - // Get latest commit date and update to current repository updated time - commitDate, err := git.GetLatestCommitTime(m.Repo.RepoPath()) + newCommitID, err := git.GetFullCommitID(gitRepo.Path, result.newCommitID) if err != nil { - log.Error("GetLatestCommitDate [%d]: %v", m.RepoID, err) + log.Error("GetFullCommitID [%d]: %v", m.RepoID, err) continue } - - if err = models.UpdateRepositoryUpdatedTime(m.RepoID, commitDate); err != nil { - log.Error("Update repository 'updated_unix' [%d]: %v", m.RepoID, err) + commits, err := gitRepo.CommitsBetweenIDs(newCommitID, oldCommitID) + if err != nil { + log.Error("CommitsBetweenIDs [repo_id: %d, new_commit_id: %s, old_commit_id: %s]: %v", m.RepoID, newCommitID, oldCommitID, err) continue } + if err = models.MirrorSyncPushAction(m.Repo, models.MirrorSyncPushActionOptions{ + RefName: result.refName, + OldCommitID: oldCommitID, + NewCommitID: newCommitID, + Commits: models.ListToPushCommits(commits), + }); err != nil { + log.Error("MirrorSyncPushAction [repo_id: %d]: %v", m.RepoID, err) + continue + } + } + + // Get latest commit date and update to current repository updated time + commitDate, err := git.GetLatestCommitTime(m.Repo.RepoPath()) + if err != nil { + log.Error("GetLatestCommitDate [%d]: %v", m.RepoID, err) + return + } + + if err = models.UpdateRepositoryUpdatedTime(m.RepoID, commitDate); err != nil { + log.Error("Update repository 'updated_unix' [%d]: %v", m.RepoID, err) + return } } diff --git a/services/mirror/mirror_test.go b/services/mirror/mirror_test.go index 9ad11b726563..37e8a7be5737 100644 --- a/services/mirror/mirror_test.go +++ b/services/mirror/mirror_test.go @@ -51,6 +51,7 @@ func TestRelease_MirrorDelete(t *testing.T) { gitRepo, err := git.OpenRepository(repoPath) assert.NoError(t, err) + defer gitRepo.Close() findOptions := models.FindReleasesOptions{IncludeDrafts: true, IncludeTags: true} initCount, err := models.GetReleaseCountByRepoID(mirror.ID, findOptions) diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go index 2872db7bd289..ca00cdaad9bc 100644 --- a/services/pull/commit_status.go +++ b/services/pull/commit_status.go @@ -55,6 +55,7 @@ func IsPullCommitStatusPass(pr *models.PullRequest) (bool, error) { if err != nil { return false, errors.Wrap(err, "OpenRepository") } + defer headGitRepo.Close() if !headGitRepo.IsBranchExist(pr.HeadBranch) { return false, errors.New("Head branch does not exist, can not merge") diff --git a/services/release/release_test.go b/services/release/release_test.go index d30dfee2865c..effab2125106 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -27,6 +27,7 @@ func TestRelease_Create(t *testing.T) { gitRepo, err := git.OpenRepository(repoPath) assert.NoError(t, err) + defer gitRepo.Close() assert.NoError(t, CreateRelease(gitRepo, &models.Release{ RepoID: repo.ID, From 3227a11f71200c6018b8af584d34fd85261de264 Mon Sep 17 00:00:00 2001 From: John Olheiser <42128690+jolheiser@users.noreply.github.com> Date: Wed, 13 Nov 2019 15:31:27 -0600 Subject: [PATCH 6/8] Backport 1.9.6 (#8969) Signed-off-by: jolheiser --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a92a61b49753..e1f09f8bbdd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -289,6 +289,14 @@ been added to each release, please refer to the [blog](https://blog.gitea.io). * wiki - editor - add buttons 'inline code', 'empty checkbox', 'checked checkbox' (#7243) * Fix Statuses API only shows first 10 statuses: Add paging and extend API GetCommitStatuses (#7141) +## [1.9.6](https://github.com/go-gitea/gitea/releases/tag/v1.9.6) - 2019-11-13 +* BUGFIXES + * Allow to merge if file path contains " or \ (#8629) (#8772) + * Fix 500 when edit hook (#8782) (#8790) + * Fix issue with user.fullname (#8904) + * Update Github Migration Test (#8897) (#8946) + * Add Close() method to gogitRepository (#8901) (#8958) + ## [1.9.5](https://github.com/go-gitea/gitea/releases/tag/v1.9.5) - 2019-10-30 * BREAKING * Hide some user information via API if user doesn't have enough permission (#8655) (#8658) From 023ae3c48cded84dee6bdcbcf7bba8cc92a4a408 Mon Sep 17 00:00:00 2001 From: guillep2k <18600385+guillep2k@users.noreply.github.com> Date: Wed, 13 Nov 2019 21:38:12 -0300 Subject: [PATCH 7/8] Hotfix for review actions and notifications (#8965) --- models/issue_comment.go | 21 ++++++++++++++++----- models/review.go | 1 + routers/repo/pull_review.go | 10 ++++++---- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index 2d5f2839bf50..f3921b80c56a 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -538,6 +538,10 @@ func sendCreateCommentAction(e *xorm.Session, opts *CreateCommentOptions, commen switch opts.Type { case CommentTypeCode: if comment.ReviewID != 0 { + // Hotfix for 1.10.0 as the Review object has not yet been committed in the other session + if opts.Review != nil { + comment.Review = opts.Review + } if comment.Review == nil { if err := comment.loadReview(e); err != nil { return err @@ -596,6 +600,12 @@ func sendCreateCommentAction(e *xorm.Session, opts *CreateCommentOptions, commen if err = opts.Issue.updateClosedNum(e); err != nil { return err } + case CommentTypeReview: + // Hotfix for 1.10.0; make sure a dashboard entry is created + if opts.Content == "" { + return nil + } + act.OpType = ActionCommentIssue } // update the issue's updated_unix column if err = updateIssueCols(e, opts.Issue, "updated_unix"); err != nil { @@ -751,11 +761,12 @@ func createIssueDependencyComment(e *xorm.Session, doer *User, issue *Issue, dep // CreateCommentOptions defines options for creating comment type CreateCommentOptions struct { - Type CommentType - Doer *User - Repo *Repository - Issue *Issue - Label *Label + Type CommentType + Doer *User + Repo *Repository + Issue *Issue + Label *Label + Review *Review DependentIssueID int64 OldMilestoneID int64 diff --git a/models/review.go b/models/review.go index 454d16ee8806..5c1449c18a9a 100644 --- a/models/review.go +++ b/models/review.go @@ -135,6 +135,7 @@ func (r *Review) publish(e *xorm.Engine) error { Repo: review.Issue.Repo, Type: comm.Type, Content: comm.Content, + Review: r, }, comm); err != nil { log.Warn("sendCreateCommentAction: %v", err) } diff --git a/routers/repo/pull_review.go b/routers/repo/pull_review.go index 5eb0dfe9a73d..4c17537071eb 100644 --- a/routers/repo/pull_review.go +++ b/routers/repo/pull_review.go @@ -174,6 +174,12 @@ func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) { return } } + + // Hotfix 1.10.0: make sure the review exists before creating the head comment + if err = review.Publish(); err != nil { + ctx.ServerError("Publish", err) + return + } comm, err := models.CreateComment(&models.CreateCommentOptions{ Type: models.CommentTypeReview, Doer: ctx.User, @@ -186,10 +192,6 @@ func SubmitReview(ctx *context.Context, form auth.SubmitReviewForm) { ctx.ServerError("CreateComment", err) return } - if err = review.Publish(); err != nil { - ctx.ServerError("Publish", err) - return - } pr, err := issue.GetPullRequest() if err != nil { From 9619ccf0e54c0f8502b5e1bc7dad4d3471330f9c Mon Sep 17 00:00:00 2001 From: John Olheiser <42128690+jolheiser@users.noreply.github.com> Date: Wed, 13 Nov 2019 23:09:58 -0600 Subject: [PATCH 8/8] Changelog for 1.10.0 (#8978) Signed-off-by: jolheiser --- CHANGELOG.md | 54 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1f09f8bbdd8..081361c36a34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,43 @@ This changelog goes through all the changes that have been made in each release without substantial changes to our git log; to see the highlights of what has been added to each release, please refer to the [blog](https://blog.gitea.io). -## [1.10.0-RC2](https://github.com/go-gitea/gitea/releases/tag/v1.10.0-rc2) - 2019-10-30 +## [1.10.0](https://github.com/go-gitea/gitea/releases/tag/v1.10.0) - 2019-11-13 * BREAKING * Fix deadline on update issue or PR via API (#8698) * Hide some user information via API if user doesn't have enough permission (#8655) (#8657) + * Remove legacy handling of drone token (#8191) + * Change repo search to use exact match for topic search. (#7941) + * Add pagination for admin api get orgs and fix only list public orgs bug (#7742) + * Implement the ability to change the ssh port to match what is in the gitea config (#7286) +* SECURITY + * Ignore mentions for users with no access (#8395) + * Be more strict with git arguments (#7715) + * reserve .well-known username (#7637) +* FEATURE + * Org/Members: display 2FA members states + optimize sql requests (#7621) + * SetDefaultBranch on pushing to empty repository (#7610) + * Adds side-by-side diff for images (#6784) + * API method to list all commits of a repository (#6408) + * Password Complexity Checks (#6230) + * Add option to initialize repository with labels (#6061) + * Add additional password hash algorithms (#6023) * BUGFIXES + * Allow to merge if file path contains " or \ (#8629) (#8771) + * On windows set core.longpaths true (#8776) (#8786) + * Fix 500 when edit hook (#8782) (#8789) + * Fix Checkbox at RepoSettings Protected Branch (#8799) (#8801) + * Fix SSH2 conditional in key parsing code (#8806) (#8810) + * Fix commit expand button to not go to commit link (#8745) (#8825) + * Fix new user form for non-local users (#8826) (#8828) + * Fix to close opened io resources as soon as not needed (#8839) (#8846) + * Fix edit content button on migrated issue content (#8877) (#8884) + * Fix require external registration password (#8885) (#8890) + * Fix password complexity check on registration (#8887) (#8888) + * Update Github Migration Tests (#8896) (#8938) (#8945) + * Fix issue with user.fullname (#8903) + * Enable punctuations ending mentions (#8889) (#8894) + * Add Close() method to gogitRepository (#8901) (#8956) + * Hotfix for review actions and notifications (#8965) * Expose db.SetMaxOpenConns and allow non MySQL dbs to set conn pool params (#8528) (#8618) * Fix milestone close timestamp (#8728) (#8730) * Fix 500 when getting user as unauthenticated user (#8653) (#8663) @@ -29,22 +61,6 @@ been added to each release, please refer to the [blog](https://blog.gitea.io). * Fix password complexity regex for special characters (#8524) * Prevent .code-view from overriding font on icon fonts (#8614) (#8627) * Allow more than 255 characters for tokens in external_login_user table (#8554) - -## [1.10.0-RC1](https://github.com/go-gitea/gitea/releases/tag/v1.10.0-rc1) - 2019-10-14 -* BREAKING - * Remove legacy handling of drone token (#8191) - * Change repo search to use exact match for topic search. (#7941) - * Add pagination for admin api get orgs and fix only list public orgs bug (#7742) - * Implement the ability to change the ssh port to match what is in the gitea config (#7286) -* FEATURE - * Org/Members: display 2FA members states + optimize sql requests (#7621) - * SetDefaultBranch on pushing to empty repository (#7610) - * Adds side-by-side diff for images (#6784) - * API method to list all commits of a repository (#6408) - * Password Complexity Checks (#6230) - * Add option to initialize repository with labels (#6061) - * Add additional password hash algorithms (#6023) -* BUGFIXES * Fix errors in create org UI regarding team access permission (#8506) * Fix bug on FindExternalUsersByProvider (#8504) * Create .ssh dir as necessary (#8486) @@ -244,10 +260,6 @@ been added to each release, please refer to the [blog](https://blog.gitea.io). * Support setting cookie domain (#6288) * Move migrating repository from frontend to backend (#6200) * Delete releases attachments if release is deleted (#6068) -* SECURITY - * Ignore mentions for users with no access (#8395) - * Be more strict with git arguments (#7715) - * reserve .well-known username (#7637) * TRANSLATION * Latvian translation for home page (#8468) * Add home template italian translation (#8352)