Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gitbase: update go-borges and support old siva files #911

Merged
merged 2 commits into from
Jul 3, 2019
Merged

gitbase: update go-borges and support old siva files #911

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a47277
gitbase: update go-borges and support old siva files
jfontan Jun 28, 2019
20247cc
Merge branch 'master' into oldsiva
ajnavarro Jul 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Now non rooted siva files support old siva rooted repositories.

## [0.22.0-rc2] - 2019-06-24

### Fixed
Expand Down
41 changes: 29 additions & 12 deletions cmd/gitbase/command/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/sirupsen/logrus"
"github.com/src-d/go-borges"
"github.com/src-d/go-borges/libraries"
"github.com/src-d/go-borges/oldsiva"
"github.com/src-d/go-borges/plain"
"github.com/src-d/go-borges/siva"
sqle "github.com/src-d/go-mysql-server"
Expand Down Expand Up @@ -227,7 +228,7 @@ func (c *Server) buildDatabase() error {

c.sharedCache = cache.NewObjectLRU(c.CacheSize * cache.MiByte)

c.rootLibrary = libraries.New(libraries.Options{})
c.rootLibrary = libraries.New(nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

This enigmatic nil looks so, so for me (and under the hood you check if it's a nil to create an empty option. Always when I see this nil in params I wonder, shall I pass sth. Or what this nil really means (defaults, nothing, etc.)

I know it's not really a part of this PR, but can we make this interface sth. like:

libraries.New().WithOptions(...)

// and maybe handy helper:
libraries.WithOption(...)

it will feel more like context.WithCancel(context.Background())
WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original plain storage used this system so we changed the rest to be more homogeneous. We can open an issue in go-borges to propose changing it. Now is the right moment to do so as we are changing the interface.

Copy link
Contributor

Choose a reason for hiding this comment

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

We have to think how to unify options in some way and we could address this also in that process.

We could add a WithOptions or SetOptions to the Library interface but I'm not sure if this is something that should belong to there.

Personally, I don't like chain functions/methods calls. I'd rather add specific constructors NewWithOptions to add custom options if you need to work with no default options.

@jfontan shall we open an issue to discuss it there?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would try to get rid of this mysterious nil
how about sth. like context interface:

lib := library.New().WithOption(...).WithSiva(...)
//or
lib = library.WithOption(library.New())
lib = library.WithSiva(lib, sivaOpts)

WDYT?

c.pool = gitbase.NewRepositoryPool(c.sharedCache, c.rootLibrary)

if err := c.addDirectories(); err != nil {
Expand Down Expand Up @@ -318,18 +319,34 @@ func (c *Server) addDirectories() error {

func (c *Server) addDirectory(d directory) error {
if d.Format == "siva" {
sivaOpts := siva.LibraryOptions{
Transactional: true,
RootedRepo: d.Rooted,
Cache: c.sharedCache,
Bucket: d.Bucket,
Performance: true,
RegistryCache: 100000,
}
var lib borges.Library
var err error

if d.Rooted {
sivaOpts := &siva.LibraryOptions{
Transactional: true,
RootedRepo: d.Rooted,
Cache: c.sharedCache,
Bucket: d.Bucket,
Performance: true,
RegistryCache: 100000,
}

lib, err := siva.NewLibrary(d.Path, osfs.New(d.Path), sivaOpts)
if err != nil {
return err
lib, err = siva.NewLibrary(d.Path, osfs.New(d.Path), sivaOpts)
Copy link
Contributor

Choose a reason for hiding this comment

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

so maybe we can combine these 2 libs, e.g.:

lib := library.New()
//....
lib = library.WithSiva(lib, sivaOpts)
// or lib = lib.WithSiva(sivaOpts)
// or lib = lib.WithSiva(...).WithOldSiva(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now there are 4 types of libraries:

  • plain: holds normal repositores
  • siva: repositories in siva files, the new format
  • oldsiva: sivas generated by borges, old format
  • libraries: a meta library that can hold any other library.

Here we are generating libraries of different types depending on the options we get and adding it to the "meta library" rootLibrary. We could have added an AddSiva or AddOldsiva to the libraries.Library structure but it's meant to hold any structure that implements the interface borges.Library. I don't think that coupling libraries.Library with the already implemented types of library adds anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is just my taste, so no worries. In this case I thought we may have WithOptions(...)
and in siva, plain, ... you have have certain type's options, so library would be top level, and all types would define specific options.

if err != nil {
return err
}
} else {
sivaOpts := &oldsiva.LibraryOptions{
Cache: c.sharedCache,
Bucket: d.Bucket,
RegistryCache: 100000,
}

lib, err = oldsiva.NewLibrary(d.Path, osfs.New(d.Path), sivaOpts)
if err != nil {
return err
}
}

err = c.rootLibrary.Add(lib)
Expand Down
4 changes: 2 additions & 2 deletions common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,14 +313,14 @@ func newMultiLibrary() (*multiLibrary, error) {
plainLib.AddLocation(plainLoc)

sivaFS := memfs.New()
sivaLib, err := siva.NewLibrary("siva", sivaFS, siva.LibraryOptions{
sivaLib, err := siva.NewLibrary("siva", sivaFS, &siva.LibraryOptions{
RootedRepo: true,
})
if err != nil {
return nil, err
}

libs := libraries.New(libraries.Options{})
libs := libraries.New(nil)

err = libs.Add(plainLib)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions database_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const testDBName = "foo"
func TestDatabase_Tables(t *testing.T) {
require := require.New(t)

lib := libraries.New(libraries.Options{})
lib := libraries.New(nil)
db := getDB(t, testDBName, NewRepositoryPool(nil, lib))

tables := db.Tables()
Expand Down Expand Up @@ -45,7 +45,7 @@ func TestDatabase_Tables(t *testing.T) {
func TestDatabase_Name(t *testing.T) {
require := require.New(t)

lib := libraries.New(libraries.Options{})
lib := libraries.New(nil)
db := getDB(t, testDBName, NewRepositoryPool(nil, lib))
require.Equal(testDBName, db.Name())
}
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require (
github.com/opentracing/opentracing-go v1.1.0
github.com/sirupsen/logrus v1.3.0
github.com/src-d/enry/v2 v2.0.0
github.com/src-d/go-borges v0.0.0-20190624135448-6ee47472d565
github.com/src-d/go-borges v0.0.0-20190628121335-da12a84d60fd
github.com/src-d/go-git v4.7.0+incompatible
github.com/src-d/go-git-fixtures v3.5.1-0.20190605154830-57f3972b0248+incompatible
github.com/src-d/go-mysql-server v0.4.1-0.20190624170509-8702d43af506
Expand Down
6 changes: 4 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,8 @@ github.com/src-d/enry/v2 v2.0.0 h1:2ADqfDHhroFwL1RGhMS9e15NkEwln8P4AABwVvIdAlo=
github.com/src-d/enry/v2 v2.0.0/go.mod h1:qQeCMRwzMF3ckeGr+h0tJLdxXnq+NVZsIDMELj0t028=
github.com/src-d/gcfg v1.4.0 h1:xXbNR5AlLSA315x2UO+fTSSAXCDf+Ar38/6oyGbDKQ4=
github.com/src-d/gcfg v1.4.0/go.mod h1:p/UMsR43ujA89BJY9duynAwIpvqEujIH/jFlfL7jWoI=
github.com/src-d/go-borges v0.0.0-20190624135448-6ee47472d565 h1:cyTdUPYEW0oGeXNCjZes3aTr+Ent3F2OHD3rm6Qy/bs=
github.com/src-d/go-borges v0.0.0-20190624135448-6ee47472d565/go.mod h1:Myl/zHrk3iT/I5T08RTBpuGzchucytSsi6p7KzM2lOA=
github.com/src-d/go-borges v0.0.0-20190628121335-da12a84d60fd h1:jUbtZFWSqGX1DfD2evCwA4y7LIrWV0W8h06sjWZANf0=
github.com/src-d/go-borges v0.0.0-20190628121335-da12a84d60fd/go.mod h1:Myl/zHrk3iT/I5T08RTBpuGzchucytSsi6p7KzM2lOA=
github.com/src-d/go-git v4.7.0+incompatible h1:IYSSnbAHeKmsfbQFi9ozbid+KNh0bKjlorMfQehQbcE=
github.com/src-d/go-git v4.7.0+incompatible/go.mod h1:1bQciz+hn0jzPQNsYj0hDFZHLJBdV7gXE2mWhC7EkFk=
github.com/src-d/go-git-fixtures v3.5.1-0.20190605154830-57f3972b0248+incompatible h1:A5bKevhs9C//Nh8QV0J+1KphEaIa25cDe1DTs/yPxDI=
Expand Down Expand Up @@ -379,6 +379,8 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=
honnef.co/go/tools v0.0.0-2019.2.1 h1:fW1wbZIKRbRK56ETe5SYloH5SdLzhXOFet2KlpRKDqg=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099 h1:XJP7lxbSxWLOMNdBE4B/STaqVy6L73o0knwj2vIlxnw=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
modernc.org/mathutil v1.0.0 h1:93vKjrJopTPrtTNpZ8XIovER7iCIH1QU7wNbOQXC60I=
modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=
Expand Down
2 changes: 1 addition & 1 deletion integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ func TestMissingHeadRefs(t *testing.T) {

path := filepath.Join(cwd, "_testdata")

lib, err := siva.NewLibrary("siva", osfs.New(path), siva.LibraryOptions{
lib, err := siva.NewLibrary("siva", osfs.New(path), &siva.LibraryOptions{
RootedRepo: true,
})
require.NoError(err)
Expand Down
8 changes: 4 additions & 4 deletions internal/rule/squashjoins_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
func TestAnalyzeSquashJoinsExchange(t *testing.T) {
require := require.New(t)

lib := libraries.New(libraries.Options{})
lib := libraries.New(nil)

catalog := sql.NewCatalog()
catalog.AddDatabase(
Expand Down Expand Up @@ -57,7 +57,7 @@ func TestAnalyzeSquashJoinsExchange(t *testing.T) {
func TestAnalyzeSquashNaturalJoins(t *testing.T) {
require := require.New(t)

lib := libraries.New(libraries.Options{})
lib := libraries.New(nil)

catalog := sql.NewCatalog()
catalog.AddDatabase(
Expand Down Expand Up @@ -2288,7 +2288,7 @@ func TestIsJoinLeafSquashable(t *testing.T) {
}

func TestOrderedTableNames(t *testing.T) {
lib := libraries.New(libraries.Options{})
lib := libraries.New(nil)
tables := gitbase.NewDatabase("foo", gitbase.NewRepositoryPool(nil, lib)).Tables()

input := []sql.Table{
Expand Down Expand Up @@ -2830,6 +2830,6 @@ func (l dummyLookup) Indexes() []string {
}

func gitbaseTables() map[string]sql.Table {
lib := libraries.New(libraries.Options{})
lib := libraries.New(nil)
return gitbase.NewDatabase("", gitbase.NewRepositoryPool(nil, lib)).Tables()
}