Check if missing/modified/unused deps in vendor and fix errors

[sapk]

I was planning to add a check in makefile for Ci to check vendoring changes because it is difficult to check all import and validate vendoring in PR. For example, the change in makefile (test) would catch any missing vendoring or vendoring without editing vendor.json or any vendor that could be removed after a change.

Doing that I found that govendor status +outside +unused returns on master :

The following packages are missing or modified locally:
	github.com/boltdb/bolt
	github.com/go-xorm/core
	github.com/smartystreets/assertions/internal/go-render/render
	github.com/smartystreets/assertions/internal/oglematchers
	github.com/smartystreets/goconvey/convey/gotest
	github.com/smartystreets/goconvey/convey/reporting
	gopkg.in/ldap.v2
Error: status failed for 7 package(s)

So I am adding fix of vendor :

• Revome un-used import github.com/smartystreets (used for tests in gogs)
• Remove github.com/boltdb/bolt unused dep Fetch github.com/boltdb/bolt
• github.com/go-xorm/core Is on a commit from a PR not from master : Fix potential data race in Column.fieldPath go-xorm/core#21 -> fetch last commit
• sync gopkg.in/ldap.v2/ldap.go since a comment seems to be different in vendor than remote source.

After rebasing and looking at failing drone I found that PR #1448 put to a5cb21c@go-xorm/xorm for files but vendor.json (https://github.com/go-gitea/gitea/blob/master/vendor/vendor.json#L460) to 7e70eb. So I update it and it is a good illustration of difficult to catch error. ^^

[sapk]

After looking further for change from github.com/boltdb/bolt base it's seems that changes were made to compile on specific platform like #1107.

Don't really know what to do so ....

Should we make a fork of bolt if we want a custom code and use it in vendor ?

[sapk]

Could we simply use custom LDFLAGS for compilation like -X github.com/boltdb/bolt.maxMapSize=0x40000000 ?

[strk]

Yes I think we need a fork for modifications in dependencies.
And I love the idea of automating the check for vendoring, so here's my LGTM shall you fall short of them.

BTW: might be worth to put this check in its own Makefile rule so in case it'll be hard to enforce (ie: needs network access and can fail for external reasons) it could easily be removed from main test rule while still existing in its own rule...

[sapk]

I use github.com/sapk-fork/bolt as source for bolt but it would be better to "host" it under gitea group after.
@tboerger I think you have to create the .sig-file for Drone like #1488

[tboerger]

The boltdb change was intentional until boltdb fixes the issue upstream.

[strk]

It makes sense to always reference an external repository with no changes, so that changes can be justified by separate development stream (tickets or what else).

[sapk]

@tboerger I understand but there is no change since 3 month and It would be more cleaner that vendor use the same version as remote (in case of sync). So using a fork in the time of the hold in bolt repo is good tempory fix. It will be easy to return to parent repo and destroy temporary fork. It will permit to check vendor in further PR in gitea.

[sapk]

Now that I am in gitea orgs can I create a repo to contain the modified version of bolt ?

[lunny]

@sapk I'm sorry you cannot. In Gitea, all the maintainers has the permissions to agree or disagree the PRs. Nobody could push a commit directly. Only Owners and Team Maintainers could create repos and merge PRs. If a new repo is needed, I could create one and you can init it before we can set the branches protected. @tboerger @strk @sapk any idea?

[strk]

My idea is that all maintainer should have more power (merge, create repos, etc.). Failing that, please owners create the bolt fork and let @sapk send the needed PR there.

[pgaskin]

I do not think maintainers should be able to merge or create repos. In my opinion, reviewing code, writing code, and managing code are different things. Someone who wrote code may not know what to do in case of a merge conflict or a broken merge. Also, many people run gitea off master (including me), so it needs to remain working with less chance of breakage. Also, imo currently there are enough people with the ability to merge.

[sapk]

@lunny I suggest that because actually on GitHub I have discovered that I have the right to create a repo under go-gitea (I don't know if it is a misconfiguration) but before doing that I would discuss it with the team members.

The goal is just to have a fork of bolt with the same additional commit that sapk-fork/bolt@606636e to be able to built for mips systems.

You can create it and I will update this PR to have this fork for bolt vendor.

[lunny]

@strk too many people has merge permission make collaboration difficult that we had tried it. I would like less people has merge permission. Currently we have 6 people has merge permission, I think it's enough! Maintainers' main work is to write PR and review PR. According to our rule, if no maintainer review a PR, it could be merged. So every PR will be reviewed by almost 3 people. That will keep the master more stable I think. Of course we ever have some breakage on the master, so if you have better idea about how to organizate the collaboration please let's discuss it on gitter.
@sapk https://github.com/go-gitea/bolt created. I will investigate the permission control.

[sapk]

@lunny thx

I migrate to this repo. Everything should be good. Just maybe need a new sig for drone.

[sapk]

@lunny like you suggest on gitter it should be better if github.com/go-gitea/bolt is a fork but not necessary. You can re-create it from github fork and I will re-add the commit.

[lunny]

@sapk done. Thanks!

[sapk]

@lunny I have not the right to push directly so I make a PR go-gitea/bolt#1 that could be merge.

[sapk]

Ok good to go. Just need one L-G-T-M.

[strk]

There you are: LGTM

[strk]

Uhm, it needs one from someone else :/

[lunny]

build failed

[sapk]

@lunny Build failed because it need a update on drone.sig. You helped on #1290 to sign the new drone file in @ab4bdd879cfa427dbed9a6616e262c260bb03595 if I understand well. Can we do the same on this PR ?

The new make test-vendor pass on drone (the only addition to drone.yml).

[strk]

Sorry this was late, but any reason why "test-vendor" is not made a dependency of "test" ? (it would also have meant no change in .drone.yml...)

[sapk]

You suggest to put it in own rule since it need internet to work.

[sapk]

Now everything is green and drone.yml is signed.

[strk]

My suggestion was:

in case it'll be hard to enforce (ie: needs network access and can fail for external reasons)
it could easily be removed from main test rule while still existing in its own rule...

So was an open possibility for the future, but I still saw it as part of the "test" rule initially (ie: a dependency). Nothing that should stop this PR from being merged.

[lunny]

LGTM

[lunny]

let L-G-T-M work