Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

dep: Add Manifest constructor #1019

Merged
merged 1 commit into from
Aug 22, 2017
Merged

dep: Add Manifest constructor #1019

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions cmd/dep/ensure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -603,9 +603,8 @@ func (cmd *ensureCommand) runAdd(ctx *dep.Ctx, args []string, p *dep.Project, sm

// Prep post-actions and feedback from adds.
var reqlist []string
appender := &dep.Manifest{
Constraints: make(gps.ProjectConstraints),
}
appender := dep.NewManifest()

for pr, instr := range addInstructions {
for path := range instr.ephReq {
reqlist = append(reqlist, path)
Expand Down
4 changes: 1 addition & 3 deletions cmd/dep/glide_importer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,7 @@ func (g *glideImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, e
task.WriteString("...")
g.logger.Println(task)

manifest := &dep.Manifest{
Constraints: make(gps.ProjectConstraints),
}
manifest := dep.NewManifest()

for _, pkg := range append(g.yaml.Imports, g.yaml.TestImports...) {
pc, err := g.buildProjectConstraint(pkg)
Expand Down
4 changes: 1 addition & 3 deletions cmd/dep/godep_importer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,7 @@ func (g *godepImporter) load(projectDir string) error {
func (g *godepImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
g.logger.Println("Converting from Godeps.json ...")

manifest := &dep.Manifest{
Constraints: make(gps.ProjectConstraints),
}
manifest := dep.NewManifest()
lock := &dep.Lock{}

for _, pkg := range g.json.Imports {
Expand Down
5 changes: 0 additions & 5 deletions cmd/dep/godep_importer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ func TestGodepConfig_Convert(t *testing.T) {
},
},
convertTestCase: &convertTestCase{

projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"),
wantConstraint: "^1.12.0-12-g2fd980e",
wantLockCount: 1,
Expand All @@ -70,7 +69,6 @@ func TestGodepConfig_Convert(t *testing.T) {
},
},
convertTestCase: &convertTestCase{

projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"),
wantConstraint: "^1.0.0",
wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"),
Expand All @@ -83,7 +81,6 @@ func TestGodepConfig_Convert(t *testing.T) {
Imports: []godepPackage{{ImportPath: ""}},
},
convertTestCase: &convertTestCase{

wantConvertErr: true,
},
},
Expand All @@ -96,7 +93,6 @@ func TestGodepConfig_Convert(t *testing.T) {
},
},
convertTestCase: &convertTestCase{

wantConvertErr: true,
},
},
Expand All @@ -116,7 +112,6 @@ func TestGodepConfig_Convert(t *testing.T) {
},
},
convertTestCase: &convertTestCase{

projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"),
wantLockCount: 1,
wantConstraint: "^1.0.0",
Expand Down
7 changes: 3 additions & 4 deletions cmd/dep/gopath_scanner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,9 @@ func (g *gopathScanner) InitializeRootManifestAndLock(rootM *dep.Manifest, rootL
return err
}

g.origM = &dep.Manifest{
Constraints: g.pd.constraints,
Ovr: make(gps.ProjectConstraints),
}
g.origM = dep.NewManifest()
g.origM.Constraints = g.pd.constraints
Copy link
Collaborator

@jmank88 jmank88 Aug 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The cases which overwrite the Constraints map in this way now have an extra initialization in the constructor. Multiple constructors or multiple optional paramaters seem like messy options. What about something like an Init() (or SetDefaults etc.) method instead, to be called after struct initialization, which only inits nil maps?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm... I thought about solutions to this but couldn't come up with anything I'm comfortable with.

Manifests are initialized once or twice per dep command, so it shouldn't have any impact on performance. Right?

Copy link
Collaborator

@jmank88 jmank88 Aug 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Won't it be O(N+1) manifests for N imported projects (and self)?
It's still not a big deal (at least not for now). My WIP cache branch messes with these a bit (since locks and manifests are serialized/deserialized), so if that worsens the situation then I can revisit it there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I always forget about importers and recursively loading dependencies.

👍


g.origL = &dep.Lock{
P: make([]gps.LockedProject, 0, len(g.pd.ondisk)),
}
Expand Down
21 changes: 8 additions & 13 deletions cmd/dep/gopath_scanner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,14 @@ func TestGopathScanner_OverlayManifestConstraints(t *testing.T) {
v1 := gps.NewVersion("v1.0.0")
v2 := gps.NewVersion("v2.0.0")
v3 := gps.NewVersion("v3.0.0")
rootM := &dep.Manifest{
Constraints: gps.ProjectConstraints{
pi1.ProjectRoot: gps.ProjectProperties{Constraint: v1},
},
}
rootM := dep.NewManifest()
rootM.Constraints[pi1.ProjectRoot] = gps.ProjectProperties{Constraint: v1}
rootL := &dep.Lock{}
origM := dep.NewManifest()
origM.Constraints[pi1.ProjectRoot] = gps.ProjectProperties{Constraint: v2}
origM.Constraints[pi2.ProjectRoot] = gps.ProjectProperties{Constraint: v3}
gs := gopathScanner{
origM: &dep.Manifest{
Constraints: gps.ProjectConstraints{
pi1.ProjectRoot: gps.ProjectProperties{Constraint: v2},
pi2.ProjectRoot: gps.ProjectProperties{Constraint: v3},
},
},
origM: origM,
origL: &dep.Lock{},
ctx: ctx,
pd: projectData{
Expand Down Expand Up @@ -79,7 +74,7 @@ func TestGopathScanner_OverlayLockProjects(t *testing.T) {
h := test.NewHelper(t)
ctx := newTestContext(h)

rootM := &dep.Manifest{}
rootM := dep.NewManifest()
pi1 := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(testProject1)}
pi2 := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(testProject2)}
v1 := gps.NewVersion("v1.0.0")
Expand All @@ -89,7 +84,7 @@ func TestGopathScanner_OverlayLockProjects(t *testing.T) {
P: []gps.LockedProject{gps.NewLockedProject(pi1, v1, []string{})},
}
gs := gopathScanner{
origM: &dep.Manifest{Constraints: make(gps.ProjectConstraints)},
origM: dep.NewManifest(),
origL: &dep.Lock{
P: []gps.LockedProject{
gps.NewLockedProject(pi1, v2, []string{}), // ignored, already exists in lock
Expand Down
8 changes: 3 additions & 5 deletions cmd/dep/root_analyzer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,7 @@ func (a *rootAnalyzer) InitializeRootManifestAndLock(dir string, pr gps.ProjectR
}

if rootM == nil {
rootM = &dep.Manifest{
Constraints: make(gps.ProjectConstraints),
Ovr: make(gps.ProjectConstraints),
}
rootM = dep.NewManifest()
}
if rootL == nil {
rootL = &dep.Lock{}
Expand Down Expand Up @@ -88,7 +85,8 @@ func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot, sup
}
}

var emptyManifest = &dep.Manifest{Constraints: make(gps.ProjectConstraints), Ovr: make(gps.ProjectConstraints)}
var emptyManifest = dep.NewManifest()

return emptyManifest, nil, nil
}

Expand Down
8 changes: 3 additions & 5 deletions cmd/dep/vndr_importer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,9 @@ func (v *vndrImporter) loadVndrFile(dir string) error {

func (v *vndrImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
var (
manifest = &dep.Manifest{
Constraints: make(gps.ProjectConstraints),
}
lock = &dep.Lock{}
err error
manifest = dep.NewManifest()
lock = &dep.Lock{}
err error
)

for _, pkg := range v.packages {
Expand Down
17 changes: 7 additions & 10 deletions cmd/dep/vndr_importer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,16 +175,13 @@ func TestVndrConfig_Import(t *testing.T) {

constraint, err := gps.NewSemverConstraint("^2.0.0")
h.Must(err)
wantM := &dep.Manifest{
Constraints: gps.ProjectConstraints{
"github.com/sdboyer/deptest": gps.ProjectProperties{
Source: "https://github.com/sdboyer/deptest.git",
Constraint: gps.Revision("3f4c3bea144e112a69bbe5d8d01c1b09a544253f"),
},
"github.com/sdboyer/deptestdos": gps.ProjectProperties{
Constraint: constraint,
},
},
wantM := dep.NewManifest()
wantM.Constraints["github.com/sdboyer/deptest"] = gps.ProjectProperties{
Source: "https://github.com/sdboyer/deptest.git",
Constraint: gps.Revision("3f4c3bea144e112a69bbe5d8d01c1b09a544253f"),
}
wantM.Constraints["github.com/sdboyer/deptestdos"] = gps.ProjectProperties{
Constraint: constraint,
}
if !reflect.DeepEqual(wantM, m) {
t.Errorf("unexpected manifest\nhave=%+v\nwant=%+v", m, wantM)
Expand Down
20 changes: 14 additions & 6 deletions manifest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ type rawProject struct {
Source string `toml:"source,omitempty"`
}

// NewManifest instantites a new manifest.
func NewManifest() *Manifest {
return &Manifest{
Constraints: make(gps.ProjectConstraints),
Ovr: make(gps.ProjectConstraints),
}
}

func validateManifest(s string) ([]error, error) {
var warns []error
// Load the TomlTree from string
Expand Down Expand Up @@ -172,12 +180,12 @@ func readManifest(r io.Reader) (*Manifest, []error, error) {
}

func fromRawManifest(raw rawManifest) (*Manifest, error) {
m := &Manifest{
Constraints: make(gps.ProjectConstraints, len(raw.Constraints)),
Ovr: make(gps.ProjectConstraints, len(raw.Overrides)),
Ignored: raw.Ignored,
Required: raw.Required,
}
m := NewManifest()

m.Constraints = make(gps.ProjectConstraints, len(raw.Constraints))
m.Ovr = make(gps.ProjectConstraints, len(raw.Overrides))
m.Ignored = raw.Ignored
m.Required = raw.Required

for i := 0; i < len(raw.Constraints); i++ {
name, prj, err := toProject(raw.Constraints[i])
Expand Down
27 changes: 11 additions & 16 deletions manifest_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,23 +62,18 @@ func TestWriteManifest(t *testing.T) {
golden := "manifest/golden.toml"
want := h.GetTestFileString(golden)
c, _ := gps.NewSemverConstraint("^0.12.0")
m := &Manifest{
Constraints: map[gps.ProjectRoot]gps.ProjectProperties{
gps.ProjectRoot("github.com/golang/dep/internal/gps"): {
Constraint: c,
},
gps.ProjectRoot("github.com/babble/brook"): {
Constraint: gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb"),
},
},
Ovr: map[gps.ProjectRoot]gps.ProjectProperties{
gps.ProjectRoot("github.com/golang/dep/internal/gps"): {
Source: "https://github.com/golang/dep/internal/gps",
Constraint: gps.NewBranch("master"),
},
},
Ignored: []string{"github.com/foo/bar"},
m := NewManifest()
m.Constraints[gps.ProjectRoot("github.com/golang/dep/internal/gps")] = gps.ProjectProperties{
Constraint: c,
}
m.Constraints[gps.ProjectRoot("github.com/babble/brook")] = gps.ProjectProperties{
Constraint: gps.Revision("d05d5aca9f895d19e9265839bffeadd74a2d2ecb"),
}
m.Ovr[gps.ProjectRoot("github.com/golang/dep/internal/gps")] = gps.ProjectProperties{
Source: "https://github.com/golang/dep/internal/gps",
Constraint: gps.NewBranch("master"),
}
m.Ignored = []string{"github.com/foo/bar"}

got, err := m.MarshalTOML()
if err != nil {
Expand Down
5 changes: 4 additions & 1 deletion project_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,13 @@ func TestFindRoot(t *testing.T) {
}

func TestProjectMakeParams(t *testing.T) {
m := NewManifest()
m.Ignored = []string{"ignoring this"}

p := Project{
AbsRoot: "someroot",
ImportRoot: gps.ProjectRoot("Some project root"),
Manifest: &Manifest{Ignored: []string{"ignoring this"}},
Manifest: m,
Lock: &Lock{},
}

Expand Down