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

status: add project status #1367

Closed
wants to merge 14 commits into from
Closed

Conversation

darkowlzz
Copy link
Collaborator

@darkowlzz darkowlzz commented Nov 10, 2017

What does this do / why do we need it?

Adds status for project arguments.

dep status github.com/foo/bar

What should your reviewer look out for in this PR?

The printed output formatting and logic of fetching all the project details.

Do you need help or clarification on anything?

Need to figure out ways to fetch some attributes like SOURCE TYPE, UPSTREAM EXISTS, UPSTREAM VERSION EXISTS using gps.

@@ -99,6 +99,12 @@ type SourceManager interface {
// repository root.
GetManifestAndLock(ProjectIdentifier, Version, ProjectAnalyzer) (Manifest, Lock, error)

// GetVcsType returns VCS Type for the provided ProjectIdentifier.
GetVcsType(ProjectIdentifier) (string, error)
Copy link
Member

Choose a reason for hiding this comment

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

s/Vcs/Source/

also, let's create the following to capture this, rather than using strings:

type SourceType uint8

const (
	InvalidSource = iota // zero value is invalid; there is no "default" type
	VcsGit
	VcsHg
	VcsBzr
	VcsSvn
)

func (st SourceType) String() string {
	switch st {
	case VcsGit:
		return "git"
	case VcsHg:
		return "hg"
	case VcsBzr:
		return "bzr"
	case VcsSvn:
		return "svn"
	default:
		return fmt.Sprintf("%v does not represent a known source type, this probably indicates a bug", st)
	}
}

we'll then also want to update the source.sourceType() method to return this uint instead of a string.

@@ -388,6 +388,10 @@ func (sg *sourceGateway) getManifestAndLock(ctx context.Context, pr ProjectRoot,
return m, l, nil
}

func (sg *sourceGateway) getVcsType() string {
Copy link
Member

Choose a reason for hiding this comment

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

s/Vcs/Source/

also should return the SourceType type described in the other comment.

SourceType string
Packages []string
ProjectImporters map[string]string
PackageImporters map[string]string
Copy link
Member

Choose a reason for hiding this comment

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

We've got project & package importers here, but not project and package imports?

ProjectImports will be able to be a []string, but PackageImports will need to be a map[string][]string. You'll want to use pkgtree.ListPackages(), TrimHiddenPackages() and then ToReachMap() to assemble the PackageImports information.

Copy link
Member

Choose a reason for hiding this comment

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

i'd also be fine with leaving this to a follow-up PR - it's more important that we get something basically functional then ship additional bits of data later on.

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 was working on adding ProjectImports, which I realized could be obtained from collectConstraints(), so I made the necessary changes in #962 . But yeah, we can add it as a follow-up PR.

Revision string
LatestAllowed string
SourceType string
Packages []string
Copy link
Member

Choose a reason for hiding this comment

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

The comment about ProjectImports and PackageImports made me realize that it would also be worth listing the ignores that are currently affecting this project - both what packages within it are ignored (that would otherwise actually be imported), and which of its imports are ignored.

This has the potential to be a little tricky, as if there's a hash mismatch (or if the user manually messed with Gopkg.lock), then it's possible that there may be some packages listed in Gopkg.lock that actually should be ignored. There's also a fair bit of work involved in actually pulling out the ignoreds, so i think it'll be fine for us to defer this to a later PR.

i've updated the spec with new lines and some comments explaining this, so we can discuss/dissect there; just noting it here.


// Get the currently selected version from from lock.
for _, pl := range p.Lock.Projects() {
if pr == pl.Ident().ProjectRoot {
Copy link
Member

Choose a reason for hiding this comment

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

best practice here would be to minimize the indent level:

if pr != pl.Ident().ProjectRoot {
	continue
}

then we can deindent the rest of the block, and the reader can immediately recognize that we're effectively just searching the slice for that single item.

}
}

fmt.Println(projStatus)
Copy link
Member

Choose a reason for hiding this comment

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

print through the ctx.Out logger


// Update local copy of the source and then fetch all the versions.
sm.SyncSourceFor(pl.Ident())
pvs, err := sm.ListVersions(pl.Ident())
Copy link
Member

Choose a reason for hiding this comment

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

in the current implementation, ListVersions() is already guaranteed to always return the latest set of versions, so the SyncSourceFor() call is unnecessary.

that may change when the caching work goes in, but we don't need to code defensively against that eventuality - we'll cross that bridge when we come to it.


rev, _, _ := gps.VersionComponentStrings(pl.Version())
// REVISION
projStatus.Revision = rev
Copy link
Member

Choose a reason for hiding this comment

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

no need for the intermediate rev var, just assign directly to projStatus.Revision

@darkowlzz darkowlzz force-pushed the status-project branch 4 times, most recently from 07a02c2 to 6157038 Compare November 16, 2017 19:00
@darkowlzz
Copy link
Collaborator Author

darkowlzz commented Nov 16, 2017

Almost there. Figured out how to gather PACKAGE IMPORTS & PROJECT IMPORTS (hopefully correct 😅). LATEST ALLOWED is the only thing left.
Once, we have all the data, I'll focus on the presentation.

@darkowlzz darkowlzz force-pushed the status-project branch 2 times, most recently from e0098dc to 4e31fe6 Compare November 17, 2017 14:58
@darkowlzz
Copy link
Collaborator Author

darkowlzz commented Nov 17, 2017

Made some optimizations like advanced collection of package reachmaps and package list, and single gps solve for all the projects' updates, to bring down the running time. Now, status of a single project or multiple projects take almost the same time (tried with 1 to 4 projects on dep itself, ~36s). It can be made concurrent to bring that down. Follow-up PR.

Sample output:

$ dep status github.com/Masterminds/vcs

PROJECT:			github.com/Masterminds/semver
VERSION:			parse-constraints-with-dash-in-pre
CONSTRAINTS:		[]
SOURCE:			github.com/Masterminds/semver
ALT SOURCE:		https://github.com/carolynvs/semver.git
PUB VERSION:		map[semver:[1.0.0 1.0.1 1.1.0 v1.1.1 v1.2.0 v1.2.1 v1.2.2 v1.2.3 v1.3.0 v1.3.1] branches:[2.x has-implied-caret master parse-constraints-with-dash-in-pre] nonsemvers:[]]
REVISION:		a93e51b5a57ef416dac8bb02d11407b6f55d8929
LATEST ALLOWED:		a93e51b5a57ef416dac8bb02d11407b6f55d8929
SOURCE TYPE:		git
PACKAGES:		github.com/Masterminds/semver
PROJECT IMPORTERS:	map[]
PACKAGE IMPORTERS:	map[]
UPSTREAM EXISTS:		true
UPSTREAM VERSION EXISTS:	true

PROJECT IMPORTERS and PACKAGE IMPORTERS are empty in this case because we don't have any dependency that uses semver project. An example for this for github.com/dgryski/go-metro would be:

PROJECT IMPORTERS:	map[github.com/jmank88/nuts:true]
PACKAGE IMPORTERS:	map[github.com/dgryski/go-metro:[github.com/jmank88/nuts/cmd/testpaths]]

CONSTRAINTS is empty for now because it depends on constraintsCollection(), which is implemented in #962 .

NEXT: Presentation improvements.

@darkowlzz darkowlzz force-pushed the status-project branch 2 times, most recently from ba7a019 to a6ca4a8 Compare November 18, 2017 20:03
@darkowlzz
Copy link
Collaborator Author

Improved the output using tabwriter and added an integration test.

I'll add the json output in a follow-up PR, avoiding huge change-set.

Example output:

$ dep status github.com/sdboyer/deptest

PROJECT:                  github.com/sdboyer/deptest
VERSION:                  v1.0.0
CONSTRAINTS:              []
SOURCE:                   github.com/sdboyer/deptest
ALT SOURCE:
PUB VERSION:              branches: master
                          nonsemvers:
                          semvers: v0.8.0, v0.8.1, v1.0.0
REVISION:                 ff2948a2ac8f538c4ecd55962e919d1e13e74baf
LATEST ALLOWED:           ff2948a2ac8f538c4ecd55962e919d1e13e74baf
SOURCE TYPE:              git
PACKAGES:                 github.com/sdboyer/deptest
PROJECT IMPORTERS:        github.com/darkowlzz/deptest-project-status, github.com/sdboyer/deptestdos
PACKAGE IMPORTERS:        github.com/sdboyer/deptest
                            github.com/darkowlzz/deptest-project-status/pkgbar
                            github.com/darkowlzz/deptest-project-status/pkgfoo
                            github.com/sdboyer/deptestdos
UPSTREAM EXISTS:          yes
UPSTREAM VERSION EXISTS:  yes

Leaving CONSTRAINTS as it is for now since its String() output would depend on the data structure, which would be changed in #962 .

@darkowlzz darkowlzz changed the title [WIP] status: add project status status: add project status Nov 18, 2017
@sdboyer sdboyer added this to the v0.4.0 milestone Nov 20, 2017
@darkowlzz
Copy link
Collaborator Author

Rebased and updated project status to print project constraints in proper table output.

CONSTRAINTS:              ^0.5.0(btb.com/x/y)
                          ^1.0.0(gh.com/f/b)

for _, pc := range pcs {
constList = append(constList, pc.String())
}
sort.Strings(constList)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This sorting would be simplified and improved with projectConstraint sort by Project implemented in #1413 .

@darkowlzz
Copy link
Collaborator Author

darkowlzz commented Dec 24, 2017

Rebased. The end result for CONSTRAINTS field would be improved by #1452, which would collect root project constraints too and #1409, which would collect applicable constraints only.

@mvdan
Copy link
Member

mvdan commented Sep 4, 2020

Dep was officially deprecated earlier this year, and the proposal to archive this repository was accepted. As such, I'm closing outstanding issues before archiving the repository. For any further comments, please use the proposal thread on the Go issue tracker. Thanks!

@mvdan mvdan closed this Sep 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants