From fac4a7ba67f9d5f6a25540a2eb247cd090d9b1bb Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Sun, 8 Oct 2017 09:32:04 +0300 Subject: [PATCH] wip Signed-off-by: Ibrahim AshShohail --- Gopkg.toml | 5 - analyzer_test.go | 2 +- internal/gps/prune.go | 5 +- manifest.go | 291 ++++++++++++++++++++++++++++------ manifest_test.go | 39 ++--- testdata/manifest/golden.toml | 7 + 6 files changed, 271 insertions(+), 78 deletions(-) diff --git a/Gopkg.toml b/Gopkg.toml index 9cd06f3234..2e181e69ab 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -22,15 +22,10 @@ [[constraint]] name = "github.com/boltdb/bolt" version = "1.0.0" - [constraint.prune] - non-go = true - go-tests = true - unused-packages = true [[constraint]] name = "github.com/jmank88/nuts" version = "0.2.0" - prune = { non-go = true, go-tests = true, unused-packages = true } [[constraint]] name = "github.com/golang/protobuf" diff --git a/analyzer_test.go b/analyzer_test.go index 4e453620a4..40b7307422 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -38,7 +38,7 @@ func TestAnalyzerDeriveManifestAndLock(t *testing.T) { t.Fatal(err) } } else { - t.Fatalf("expected %s\n got %s", want, string(got)) + t.Fatalf("(WNT):\n%s\n(GOT):\n%s", want, string(got)) } } diff --git a/internal/gps/prune.go b/internal/gps/prune.go index b67e01b1ff..7fdff4f053 100644 --- a/internal/gps/prune.go +++ b/internal/gps/prune.go @@ -16,9 +16,12 @@ import ( // PruneOptions represents the pruning options used to write the dependecy tree. type PruneOptions uint8 +// PruneProjectOptions is map of prune options per project name. +type PruneProjectOptions map[ProjectRoot]PruneOptions + const ( // PruneNestedVendorDirs indicates if nested vendor directories should be pruned. - PruneNestedVendorDirs = 1 << iota + PruneNestedVendorDirs PruneOptions = 1 << iota // PruneUnusedPackages indicates if unused Go packages should be pruned. PruneUnusedPackages // PruneNonGoFiles indicates if non-Go files should be pruned. diff --git a/manifest.go b/manifest.go index 7f33118cf2..9fc1a442c8 100644 --- a/manifest.go +++ b/manifest.go @@ -23,29 +23,39 @@ const ManifestName = "Gopkg.toml" // Errors var ( - errInvalidConstraint = errors.New("\"constraint\" must be a TOML array of tables") - errInvalidOverride = errors.New("\"override\" must be a TOML array of tables") - errInvalidRequired = errors.New("\"required\" must be a TOML list of strings") - errInvalidIgnored = errors.New("\"ignored\" must be a TOML list of strings") - errInvalidPrune = errors.New("\"prune\" must be a TOML table of boolean values") - errInvalidProjectRoot = errors.New("ProjectRoot name validation failed") + errInvalidConstraint = errors.Errorf("%q must be a TOML array of tables", "constraint") + errInvalidOverride = errors.Errorf("%q must be a TOML array of tables", "override") + errInvalidRequired = errors.Errorf("%q must be a TOML list of strings", "required") + errInvalidIgnored = errors.Errorf("%q must be a TOML list of strings", "ignored") + errInvalidPrune = errors.Errorf("%q must be a TOML table of booleans", "prune") + + errInvalidProjectRoot = errors.New("ProjectRoot name validation failed") + errInvalidPruneValue = errors.New("prune options values must be booleans") + errInvalidPruneProject = errors.Errorf("%q must be a TOML array of tables", "prune.project") + errPruneSubProject = errors.New("prune projects should not contain sub projects") + + errInvalidRootPruneValue = errors.New("root prune options must be omitted instead of being set to false") + errInvalidPruneProjectName = errors.Errorf("%q in %q must be a string", "name", "prune.project") ) // Manifest holds manifest file data and implements gps.RootManifest. type Manifest struct { - Constraints gps.ProjectConstraints - Ovr gps.ProjectConstraints - Ignored []string - Required []string - PruneOptions gps.PruneOptions + Constraints gps.ProjectConstraints + Ovr gps.ProjectConstraints + + Ignored []string + Required []string + + PruneOptions gps.PruneOptions + PruneProjectOptions gps.PruneProjectOptions } type rawManifest struct { - Constraints []rawProject `toml:"constraint,omitempty"` - Overrides []rawProject `toml:"override,omitempty"` - Ignored []string `toml:"ignored,omitempty"` - Required []string `toml:"required,omitempty"` - PruneOptions rawPruneOptions `toml:"prune,omitempty"` + Constraints []rawProject `toml:"constraint,omitempty"` + Overrides []rawProject `toml:"override,omitempty"` + Ignored []string `toml:"ignored,omitempty"` + Required []string `toml:"required,omitempty"` + PruneOptions rawRootPruneOptions `toml:"prune,omitempty"` } type rawProject struct { @@ -56,12 +66,30 @@ type rawProject struct { Source string `toml:"source,omitempty"` } -type rawPruneOptions struct { +type rawRootPruneOptions struct { + UnusedPackages bool `toml:"unused-packages,omitempty"` + NonGoFiles bool `toml:"non-go,omitempty"` + GoTests bool `toml:"go-tests,omitempty"` + + Projects []rawPruneProject `toml:"project,omitempty"` +} + +type rawPruneProject struct { + Name string `toml:"name"` + Options rawPruneProjectOptions `toml:"options,omitempty"` +} +type rawPruneProjectOptions struct { UnusedPackages bool `toml:"unused-packages,omitempty"` NonGoFiles bool `toml:"non-go,omitempty"` GoTests bool `toml:"go-tests,omitempty"` } +const ( + pruneOptionUnusedPackages = "unused-packages" + pruneOptionGoTests = "go-tests" + pruneOptionNonGo = "non-go" +) + // NewManifest instantites a new manifest. func NewManifest() *Manifest { return &Manifest{ @@ -161,20 +189,10 @@ func validateManifest(s string) ([]error, error) { } } case "prune": - // Check if prune is of Map type - if reflect.TypeOf(val).Kind() != reflect.Map { - warns = append(warns, errors.New("prune should be a TOML table")) - } - for key, value := range val.(map[string]interface{}) { - switch key { - case "non-go", "go-tests", "unused-packages": - if _, ok := value.(bool); !ok { - warns = append(warns, errors.Errorf("unexpected type for prune.%s: %T", key, value)) - return warns, errInvalidPrune - } - default: - warns = append(warns, errors.Errorf("unknown field: prune.%s", key)) - } + pruneWarns, err := validatePrune(val) + warns = append(warns, pruneWarns...) + if err != nil { + return warns, err } default: warns = append(warns, fmt.Errorf("unknown field in manifest: %v", prop)) @@ -184,6 +202,82 @@ func validateManifest(s string) ([]error, error) { return warns, nil } +func validatePrune(val interface{}) (warns []error, err error) { + // Validate root options. + warns, err = validatePruneOptions(val, true) + if err != nil { + return nil, err + } + + // Check for redundant options + + return warns, err +} + +func validatePruneOptions(val interface{}, root bool) (warns []error, err error) { + if reflect.TypeOf(val).Kind() != reflect.Map { + return warns, errInvalidPrune + } + + for key, value := range val.(map[string]interface{}) { + switch key { + case pruneOptionNonGo, pruneOptionGoTests, pruneOptionUnusedPackages: + if option, ok := value.(bool); !ok { + return warns, errInvalidPruneValue + } else if root && !option { + return warns, errInvalidRootPruneValue + } + case "name": + if root { + warns = append(warns, errors.Errorf("%q should not include a name", "prune")) + } else if _, ok := value.(string); !ok { + return warns, errInvalidPruneProjectName + } + case "project": + if !root { + return warns, errPruneSubProject + } + if reflect.TypeOf(value).Kind() != reflect.Slice { + return warns, errInvalidPruneProject + } + for _, project := range value.([]interface{}) { + projectWarns, err := validatePruneOptions(project, false) + warns = append(warns, projectWarns...) + if err != nil { + return nil, err + } + } + + default: + if root { + warns = append(warns, errors.Errorf("unknown field %q in %q", key, "prune")) + } else { + warns = append(warns, errors.Errorf("unknown field %q in %q", key, "prune.project")) + } + } + } + + return warns, err +} + +func checkRedundantPruneOptions(raw rawManifest) (warns []error) { + rootOptions := raw.PruneOptions + + for _, project := range raw.PruneOptions.Projects { + if rootOptions.GoTests && project.Options.GoTests { + warns = append(warns, errors.Errorf("redundant prune option %q set for %q", pruneOptionGoTests, project.Name)) + } + if rootOptions.NonGoFiles && project.Options.NonGoFiles { + warns = append(warns, errors.Errorf("redundant prune option %q set for %q", pruneOptionNonGo, project.Name)) + } + if rootOptions.UnusedPackages && project.Options.UnusedPackages { + warns = append(warns, errors.Errorf("redundant prune option %q set for %q", pruneOptionUnusedPackages, project.Name)) + } + } + + return warns +} + // ValidateProjectRoots validates the project roots present in manifest. func ValidateProjectRoots(c *Ctx, m *Manifest, sm gps.SourceManager) error { // Channel to receive all the errors @@ -209,6 +303,10 @@ func ValidateProjectRoots(c *Ctx, m *Manifest, sm gps.SourceManager) error { wg.Add(1) go validate(pr) } + for pr := range m.PruneProjectOptions { + wg.Add(1) + go validate(pr) + } wg.Wait() close(errorCh) @@ -279,17 +377,41 @@ func fromRawManifest(raw rawManifest) (*Manifest, error) { m.Ovr[name] = prj } - if raw.PruneOptions.UnusedPackages { - m.PruneOptions |= gps.PruneUnusedPackages + m.PruneOptions, m.PruneProjectOptions = fromRawRootPruneOptions(raw.PruneOptions) + + return m, nil +} + +func fromRawRootPruneOptions(raw rawRootPruneOptions) (gps.PruneOptions, gps.PruneProjectOptions) { + rootOptions := gps.PruneNestedVendorDirs + pruneProjects := make(gps.PruneProjectOptions) + + if raw.UnusedPackages { + rootOptions |= gps.PruneUnusedPackages } - if raw.PruneOptions.GoTests { - m.PruneOptions |= gps.PruneGoTestFiles + if raw.GoTests { + rootOptions |= gps.PruneGoTestFiles } - if raw.PruneOptions.NonGoFiles { - m.PruneOptions |= gps.PruneNonGoFiles + if raw.NonGoFiles { + rootOptions |= gps.PruneNonGoFiles } - return m, nil + for _, p := range raw.Projects { + pr := gps.ProjectRoot(p.Name) + pruneProjects[pr] = gps.PruneNestedVendorDirs + + if raw.UnusedPackages { + pruneProjects[pr] |= gps.PruneUnusedPackages + } + if raw.GoTests { + pruneProjects[pr] |= gps.PruneGoTestFiles + } + if raw.NonGoFiles { + pruneProjects[pr] |= gps.PruneNonGoFiles + } + } + + return rootOptions, pruneProjects } // toProject interprets the string representations of project information held in @@ -323,17 +445,27 @@ func toProject(raw rawProject) (n gps.ProjectRoot, pp gps.ProjectProperties, err } pp.Source = raw.Source + return n, pp, nil } +// MarshalTOML serializes this manifest into TOML via an intermediate raw form. +func (m *Manifest) MarshalTOML() ([]byte, error) { + raw := m.toRaw() + result, err := toml.Marshal(raw) + return result, errors.Wrap(err, "Unable to marshal the lock to a TOML string") +} + // toRaw converts the manifest into a representation suitable to write to the manifest file func (m *Manifest) toRaw() rawManifest { raw := rawManifest{ - Constraints: make([]rawProject, 0, len(m.Constraints)), - Overrides: make([]rawProject, 0, len(m.Ovr)), - Ignored: m.Ignored, - Required: m.Required, + Constraints: make([]rawProject, 0, len(m.Constraints)), + Overrides: make([]rawProject, 0, len(m.Ovr)), + Ignored: m.Ignored, + Required: m.Required, + PruneOptions: rawRootPruneOptions{}, } + for n, prj := range m.Constraints { raw.Constraints = append(raw.Constraints, toRawProject(n, prj)) } @@ -344,6 +476,24 @@ func (m *Manifest) toRaw() rawManifest { } sort.Sort(sortedRawProjects(raw.Overrides)) + if (m.PruneOptions & gps.PruneNonGoFiles) == gps.PruneNonGoFiles { + raw.PruneOptions.NonGoFiles = true + } + if (m.PruneOptions & gps.PruneGoTestFiles) == gps.PruneGoTestFiles { + raw.PruneOptions.GoTests = true + } + if (m.PruneOptions & gps.PruneUnusedPackages) == gps.PruneUnusedPackages { + raw.PruneOptions.UnusedPackages = true + } + + if len(m.PruneProjectOptions) > 0 { + raw.PruneOptions.Projects = make([]rawPruneProject, 0, len(m.PruneProjectOptions)) + for name, opts := range m.PruneProjectOptions { + raw.PruneOptions.Projects = append(raw.PruneOptions.Projects, toRawPruneProject(m.PruneOptions, name, opts)) + } + sort.Sort(sortedRawPruneProjects(raw.PruneOptions.Projects)) + } + return raw } @@ -364,13 +514,6 @@ func (s sortedRawProjects) Less(i, j int) bool { return l.Source < r.Source } -// MarshalTOML serializes this manifest into TOML via an intermediate raw form. -func (m *Manifest) MarshalTOML() ([]byte, error) { - raw := m.toRaw() - result, err := toml.Marshal(raw) - return result, errors.Wrap(err, "Unable to marshal the lock to a TOML string") -} - func toRawProject(name gps.ProjectRoot, project gps.ProjectProperties) rawProject { raw := rawProject{ Name: string(name), @@ -398,6 +541,48 @@ func toRawProject(name gps.ProjectRoot, project gps.ProjectProperties) rawProjec // Has to be a semver range. raw.Version = project.Constraint.ImpliedCaretString() } + + return raw +} + +type sortedRawPruneProjects []rawPruneProject + +func (s sortedRawPruneProjects) Len() int { return len(s) } +func (s sortedRawPruneProjects) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +func (s sortedRawPruneProjects) Less(i, j int) bool { + return s[i].Name < s[j].Name +} + +func toRawPruneProject(rootOptions gps.PruneOptions, name gps.ProjectRoot, projectOptions gps.PruneOptions) rawPruneProject { + raw := rawPruneProject{ + Name: string(name), + } + + // TODO(ibrasho): don't write ineffectual options + if (projectOptions & gps.PruneNonGoFiles) == gps.PruneNonGoFiles { + if (rootOptions & gps.PruneNonGoFiles) != gps.PruneNonGoFiles { + raw.Options.NonGoFiles = true + } + } else if (rootOptions & gps.PruneNonGoFiles) == gps.PruneNonGoFiles { + raw.Options.NonGoFiles = false + } + + if (projectOptions & gps.PruneGoTestFiles) == gps.PruneGoTestFiles { + if (rootOptions & gps.PruneGoTestFiles) != gps.PruneGoTestFiles { + raw.Options.GoTests = true + } + } else if (rootOptions & gps.PruneGoTestFiles) == gps.PruneGoTestFiles { + raw.Options.GoTests = false + } + + if (projectOptions & gps.PruneUnusedPackages) == gps.PruneUnusedPackages { + if (rootOptions & gps.PruneUnusedPackages) != gps.PruneUnusedPackages { + raw.Options.UnusedPackages = true + } + } else if (rootOptions & gps.PruneUnusedPackages) == gps.PruneUnusedPackages { + raw.Options.UnusedPackages = false + } + return raw } @@ -451,3 +636,11 @@ func (m *Manifest) RequiredPackages() map[string]bool { return mp } + +func (m *Manifest) PruneOptionsFor(pr gps.ProjectRoot) gps.PruneOptions { + if po, ok := m.PruneProjectOptions[pr]; ok { + return po + } + + return m.PruneOptions +} diff --git a/manifest_test.go b/manifest_test.go index 30bfff7c29..3fb7d070d3 100644 --- a/manifest_test.go +++ b/manifest_test.go @@ -44,7 +44,11 @@ func TestReadManifest(t *testing.T) { Constraint: gps.NewBranch("master"), }, }, - Ignored: []string{"github.com/foo/bar"}, + Ignored: []string{"github.com/foo/bar"}, + PruneOptions: gps.PruneNestedVendorDirs | gps.PruneNonGoFiles, + PruneProjectOptions: gps.PruneProjectOptions{ + gps.ProjectRoot("github.com/golang/dep"): gps.PruneNestedVendorDirs, + }, } if !reflect.DeepEqual(got.Constraints, want.Constraints) { @@ -78,6 +82,11 @@ func TestWriteManifest(t *testing.T) { } m.Ignored = []string{"github.com/foo/bar"} + m.PruneOptions = gps.PruneNestedVendorDirs | gps.PruneNonGoFiles + m.PruneProjectOptions = gps.PruneProjectOptions{ + gps.ProjectRoot("github.com/golang/dep"): gps.PruneNestedVendorDirs, + } + got, err := m.MarshalTOML() if err != nil { t.Fatalf("error while marshaling valid manifest to TOML: %q", err) @@ -89,7 +98,7 @@ func TestWriteManifest(t *testing.T) { t.Fatal(err) } } else { - t.Errorf("valid manifest did not marshal to TOML as expected:\n\t(GOT): %s\n\t(WNT): %s", string(got), want) + t.Errorf("valid manifest did not marshal to TOML as expected:\n(GOT):\n%s\n(WNT):\n%s", string(got), want) } } } @@ -382,31 +391,17 @@ func TestValidateManifest(t *testing.T) { wantError: nil, }, { - name: "valid prune options in constraint", - tomlString: ` - [[constraint]] - name = "github.com/foo/bar" - version = "1.0.0" - [constraint.prune] - non-go = true - `, - wantWarn: []error{ - errors.New("invalid key \"prune\" in \"constraint\""), - }, - wantError: nil, - }, - { - name: "valid inline prune options in constraint", + name: "invalid root prune options", tomlString: ` [[constraint]] name = "github.com/foo/bar" version = "1.0.0" - prune = { non-go = true , go-tests = true } + + [prune] + non-go = false `, - wantWarn: []error{ - errors.New("invalid key \"prune\" in \"constraint\""), - }, - wantError: nil, + wantWarn: []error{}, + wantError: errInvalidRootPruneValue, }, } diff --git a/testdata/manifest/golden.toml b/testdata/manifest/golden.toml index 8614f1fdd7..4c96715035 100644 --- a/testdata/manifest/golden.toml +++ b/testdata/manifest/golden.toml @@ -12,3 +12,10 @@ ignored = ["github.com/foo/bar"] branch = "master" name = "github.com/golang/dep/internal/gps" source = "https://github.com/golang/dep/internal/gps" + +[prune] + non-go = true + + [[prune.project]] + name = "github.com/golang/dep" + non-go = false