New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support finer control for assigning fetchers to remotes. #2018

Closed
jsirois opened this Issue Aug 19, 2015 · 2 comments

Comments

Projects
None yet
2 participants
@jsirois
Copy link
Member

jsirois commented Aug 19, 2015

The rough sketch idea is 2 part:

  1. Introduce [fetchers] fetchers which is a new dict config of name (will be used as an options scope) -> FetcherType, eg:
[fetchers]
fetchers: {
    'artifactory.com': 'ArchiveFetcher',
    'github.com': 'ArchiveFetcher'
  }

The existing [fetchers] mapping becomes an ordered mapping from regex to fetcher name, eg:

[fetchers]
mapping: [
    ('github.com/gorilla', 'github.com'),
    ('.*', 'artifactory.com')
  ]

The ordering allows a dev to put mappings for the new stuff they want to try at the top of the list.

The Fetchers class would then instantiate Fetchers that are Subsystems using their names as their option scope. This allows having 2 ArchiveFetcher instances configured differently - one pointed at github.com, one at artifactory.

This grew out of discussion here: https://rbcommons.com/s/twitter/r/2655/

@jsirois

This comment has been minimized.

Copy link
Member

jsirois commented Aug 20, 2015

Turns out this was not needed for #2004 - pushing back on the stack for now.

@jsirois jsirois removed their assignment May 24, 2016

benjyw added a commit that referenced this issue May 28, 2016

Refactor and simplify go fetcher code.
This is not a small change... It gets rid of a lot of complex logic
that, with hindsight, doesn't appear to be necessary. It replaces
that complexity with a more simple approach.

This mirrors the direction that Go itself is moving in: Go's heuristics
for fetching remote deps (including the meta tag protocol) are becoming
increasingly standard. In fact, there are comments in Go's codebase(1)
to the effect that they encourage code-hosting sites to support the
meta tag protocol so that Go can remove its own hard-coded special cases.

Fetcher Types
-------------
Under this change there are two Fetcher types: The existing ArchiveFetcher,
and a new CloningFetcher.

ArchiveFetcher handles special cases where we know how to map an import
path to a tarball path.  This is useful for sites like github.com, which
do not currently support the meta tag protocol, and also for diverting
fetches to an internal artifactory, for repos that wish to do so.

CloningFetcher implements (a useful subset of) the standard Go heuristics:
It checks meta tags, and then clones the remote repo and sets its state
to the specified rev (currently this only works for git).

All remote fetches that don't map to an ArchiveFetcher will use the
CloningFetcher. This makes it trivial to use standard git-based remote
repos without any extra config.  In particular, gopkg.in, golang.org/x
and google.golang.org now support the meta tag protocol, so there is
no need to special-case them.

The Fetcher class API has also changed a bit. Now a Fetcher encapsulates
the import_path its fetching, so a new Fetcher instance is created for
each fetch operation. The Fetcher classes use Subsystems, but are no longer
themselves subsystems.

Subsystems
----------
For separation of concerns, this commit introduces two utility subsystems
that do what their names imply: GoImportMetaTagReader and ArchiveRetriever.
The fetchers use these.  Note that they are not currently unit-tested,
because by the time you mock out the network stuff, there's not that
much left. However they are indirectly tested via several other unit and
integration tests.

FetcherFactory
--------------
This change gets rid of the fetcher advertisement+registration mechanism,
which with hindsight seems like overkill, given that we expect that the
current two Fetcher classes are all we're likely to need for the forseeable
future.  The Fetchers class is gone. In its stead is much simpler
FetcherFactory subsystem. It now encapsulates the matcher logic, and uses
that to choose a fetcher.

This resolves #3439 and
#3427, and obsoletes
#2018, as we're now
going in a different, simpler, direction.

I can't add dbentley and yujie here because they aren't reviewers on RB, but
I  will ask for their review feedback via email.

(1) https://github.com/golang/go/blob/7bc40ffb05d8813bf9b41a331b45d37216f9e747/src/cmd/go/vcs.go#L874

Testing Done:
CI passes after all merges: http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3458/

Added various remote deps to one of the example targets and verified manually that it buildgens and compiles. Also added it to the integration test.

Bugs closed: 2018, 3427, 3439

Reviewed at https://rbcommons.com/s/twitter/r/3902/
@benjyw

This comment has been minimized.

Copy link
Contributor

benjyw commented May 28, 2016

Obsoleted by aa9b358.

@benjyw benjyw closed this May 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment