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

Simple approach to finding project root dir #6

Merged
merged 2 commits into from
Oct 18, 2016
Merged

Simple approach to finding project root dir #6

merged 2 commits into from
Oct 18, 2016

Conversation

sdboyer
Copy link
Member

@sdboyer sdboyer commented Oct 18, 2016

Basically, walk up the dir tree, looking for a manifest.

Project root inference is done by searching for an appropriately-named
file (currently manifest.json). Search should generally proceed from the
cwd upwards towards root.
@freeformz
Copy link

freeformz commented Oct 18, 2016

Looks good but I think at some point we should stop looking upwards once we reach the src dir of the $GOPATH entry we're in. Mostly because of a situation like this:

$ export GOPATH=$HOME/go

$ ls $HOME
go
manifest.json

$ cd $GOPATH/src/github.com/foo/bar
$ ls 
main.go

^^ Assuming there isn't a manfiest.json in any of $GOPATH/src/github.com/foo $HOME/manifest.json will be found, so the project root will be $HOME.

@sdboyer
Copy link
Member Author

sdboyer commented Oct 18, 2016

Ah! good call. That also implies another check, I think - that the tool can't work operate at all if it's off-GOPATH. I don't know how much I like that, but it seems necessary to include if we're going to limit the upwards search path to GOPATH.

If we go that far, it also might suggest that, in #7, we pick the GOPATH used for the cachedir based on the GOPATH in which we're currently situated, rather than just grabbing the first one.

(Side note - I do wish we were moving further from GOPATH rather than binding more tightly, but this is clearly the path of least resistance rn)

@freeformz
Copy link

this is why I included"at some point". Basically I think this is fine for now as long as we are aware of the caveat that this can escape upwards to a location we didn't intend.

@freeformz
Copy link

Another possible way to handle this w/o involving $GOPATH is to search upwards for directories that contain a VCS control dir/file && a manifest file.

@sdboyer
Copy link
Member Author

sdboyer commented Oct 18, 2016

this is why I included"at some point".

fair nuff 😄

Another possible way to handle this w/o involving $GOPATH is to search upwards for directories that contain a VCS control dir/file && a manifest file.

yeah, i gave some thought to that. i decided to leave that as a separate path, possibly to be used by init. it's not without its own issues, though - plenty of folks have $HOME/.git.

i figure this does best with KISS, then figuring out adaptive strategies as we run into issues.

@freeformz
Copy link

True. I don't think any approach is "fool" proof. But $HOME/.git and $HOME/manfiest.json is less likely. Anyway, this discussion is likely premature.

@sdboyer sdboyer merged commit ce9692f into golang:master Oct 18, 2016
zbintliff added a commit to zbintliff/dep that referenced this pull request Mar 3, 2017
ibrasho pushed a commit to ibrasho-forks/dep that referenced this pull request May 10, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants