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

dep: locate project by VCS root instead of manifest file #11

Closed
wants to merge 1 commit into from
Closed

Conversation

adg
Copy link
Contributor

@adg adg commented Nov 16, 2016

Also tidy up some error reporting, and fix the manifest test (some names
were unexported but the tests were not updated accordingly).

Also tidy up some error reporting, and fix the manifest test (some names
were unexported but the tests were not updated accordingly).
@sdboyer
Copy link
Member

sdboyer commented Nov 21, 2016

i'm a bit confused by this. @freeformz explained to me that you wrote this if only as a way of getting the tool to stop complaining about / not containing a manifest. which makes sense, but...

i thought, in one of our meetings a couple weeks ago, we came to the idea that while the project root == vcs root thing is important when it comes to dealing with dependencies, it's actually irrelevant when dealing with the current project. (my memory is of @adg having a literal chin-stroking epiphany moment about it, actually). looking back over the spec doc, though, we only took out reference to it in one place, and it's still present elsewhere.

IMO, looking for a manifest file to locate current project root is a lot cleaner:

  • it's exactly one filename to search upwards for. compare to multiple metadata dirs for different VCSses, difference in how those metadata dirs work for svn vs. git/hg/bzr, and the fact that we're binding the tool to one of those VCSes to make it work.
    • it'd be impossible to, say, run some kind of dep check (which we've discussed, though it's not in the spec atm) on a build server that just had a code tree w/out VCS
  • manifests have the same nesting issues as dvcs metadata dirs - if you've got one project inside another, a climbing search may find one that's higher than the user intended.
    • this same risk exists with VCS, and is actually much more likely - $HOME/.git exists on plenty of machines
    • the only substantive case where this would probably happen is if you're cd'd into your project's vendor dir and looking at projects in there which have their own manifests. but that's easy to detect, and hop the vendor dir back up to the rootmost manifest.

freeformz pushed a commit that referenced this pull request Nov 30, 2016
This is mostly the non controversial changes from #11, plus a more to
the std lib stub function.
@freeformz
Copy link

Closing this as we decided not to use VCS as per the doc. Other changes when in via #17.

@freeformz freeformz closed this Nov 30, 2016
@sdboyer sdboyer deleted the hack branch January 1, 2017 00:32
zbintliff added a commit to zbintliff/dep that referenced this pull request Mar 3, 2017
This is mostly the non controversial changes from golang#11, plus a more to
the std lib stub function.
krisnova pushed a commit to krisnova/dep that referenced this pull request Apr 21, 2017
krisnova pushed a commit to krisnova/dep that referenced this pull request Apr 21, 2017
This is mostly the non controversial changes from golang#11, plus a more to
the std lib stub function.
ibrasho pushed a commit to ibrasho-forks/dep that referenced this pull request May 10, 2017
This is mostly the non controversial changes from golang#11, plus a more to
the std lib stub function.
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.

3 participants