Skip to content
This repository has been archived by the owner on Feb 3, 2018. It is now read-only.

Introduce source concept within SourceManager #83

Merged
merged 71 commits into from
Aug 16, 2016
Merged

Conversation

sdboyer
Copy link
Owner

@sdboyer sdboyer commented Aug 1, 2016

This introduces a source concept (perhaps will be exported eventually), around which the SourceMgr operates. It replaces the old projectManager concept.

This carries with it a ton of changes with respect to deduction around import paths, repo storage, isolation of different upstream URLs (per #27), and how we deal with import paths/repos/sources/names in general (#10). The visible difference is mostly that we change SourceManager to take a ProjectIdentifier basically everywhere that it used to take a ProjectRoot.

This list of TODOs is sloppy, as a lot of discovery caused this issue to morph a few times while I was working on it. I'm trying to keep it at least semi-sane.

  • Keeping separate storage for different URLs, a la Handling slightly different URLs to same project #27
  • Introduce the concept of a Source, which is the unit that the SourceMgr actually works with
  • Moving URL selection and folding behavior into the SourceManager itself
  • Refactoring the SourceMgr internals to totally not care about anything like GOPATH at all anymore
  • Introduce baseVCSSource as embeddable helper for all vcsSources
  • Completely remove projectManager, replacing with the source system
  • Use futures to handle deferred source creation
  • Convert existing remote repo deduction tests to new deduction system
  • Add test for source unification after e.g. vanity import base URL transformation

I pulled this out a while back, but going back to it's been a long time
coming. Not all the SourceManager methods strictly need the information
in a ProjectIdentifier, but it's much easier to be consistent and just
always require it.

This does not actually convert function/method bodies - just signatures.
In no way does this come even close to compiling.
@sdboyer sdboyer added this to the MVP milestone Aug 1, 2016
@sdboyer sdboyer self-assigned this Aug 1, 2016
This strategy isn't perfect, but there's more refactoring needed for
this segment to really make it sane.
Yeah, we're just gonna have to go whole hog on this. The system's too
borky and complected as-is to sanely do a minor refactor.
There needs to be a mutex *somewhere* around these maps. These are there
as a warning reminder that something needs to be done, even though a
per-map mutex system seems very unlikely to have sufficient performance
benefit to offset the complexity cost (and concomitant deadlock risk).
This makes actually collecting the result slice more straightforward
later.
Also unexport its fields. We can revisit this later when we're
actually ready to start dealing with persisting the caches to disk.
This will hold the pathDeducers, to be used by the SourceManager when
performing path deduction.
@sdboyer
Copy link
Owner Author

sdboyer commented Aug 11, 2016

An issue exists that I'm going to punt to a followup: for import paths where deduction results in fundamentally altering the base URL (so, all vanity imports), if underlying URL is ever explicitly referenced in a different import path, then two instances of the source will end up being created in the manager, each with their own locks, but both pointing to the same actual repo on disk.

I'm leaving it in because:

  • this issue is already huge enough
  • i don't think it's very likely we'll encounter this problem in the near term
  • the fix probably involves the more significant refactor that will be an explicit API for determining URL equivalencies

This adds one additional field, a flag indicating whether the root
future is *actually* async (there's currently only one case in deduction
where it actually does require network activity). This allows the main
handling process to be a little smarter about how it stores the
information.
This helps resolve a problem that will probably only exist for vcs-type
sources, where there could be a difference between the input ident
(e.g., a plain import path) and the actual on-disk ident of the
resulting source (e.g., a full URL). The manager needs to know which
unique string ident is in use, because that will become an access path
for lookups (in addition to the input path).
Forgot to include this on the earlier commit that privileged hg over
git.
This is a major milestone. The old way of handling source information,
the projectManager, is totally gone, replaced by the new source-based
system. There's still work to be done there, but with tests passing,
we're more or less at parity with what we had before.
@sdboyer
Copy link
Owner Author

sdboyer commented Aug 15, 2016

haha, just kidding - i actually fixed that issue with the underlying URLs. it still does need a test, though.

@sdboyer
Copy link
Owner Author

sdboyer commented Aug 15, 2016

uuuuuugh NuGet

@sdboyer sdboyer added this to the v0.10.0 milestone Aug 15, 2016
This entails updating the bridge, which entails updating the solver. It
also entails updating the testing depspec source manager. All are in
alignment now, and tests are passing, so we're fully converted to the
new source-based system for gathering information (woohoo!).
@sdboyer sdboyer merged commit 737841a into master Aug 16, 2016
@sdboyer sdboyer deleted the url-separation branch September 22, 2016 03:01
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.

2 participants