apply insteadof on Remote.{fetch,pull,list} functions -- breaking change(?)

config/config.go

@@ -351,7 +351,7 @@ func (c *Config) unmarshalRemotes() error {
 
 	// Apply insteadOf url rules
 	for _, r := range c.Remotes {
-		r.applyURLRules(c.URLs)
+		r.ApplyURLRules(c.URLs)
 	}
 
 	return nil
@@ -583,6 +583,7 @@ type RemoteConfig struct {
 
 	// insteadOfRulesApplied have urls been modified
 	insteadOfRulesApplied bool
+
 	// originalURLs are the urls before applying insteadOf rules
 	originalURLs []string
 
@@ -677,19 +678,18 @@ func (c *RemoteConfig) IsFirstURLLocal() bool {
 	return url.IsLocalEndpoint(c.URLs[0])
 }
 
-func (c *RemoteConfig) applyURLRules(urlRules map[string]*URL) {
-	// save original urls
-	originalURLs := make([]string, len(c.URLs))
-	copy(originalURLs, c.URLs)
+// ApplyURLRules() updates c.URLs by substituting the longest matching insteadOf value found in urlRules.
+func (c *RemoteConfig) ApplyURLRules(urlRules map[string]*URL) {
+	// save original urls if we haven't already
+	// never overwrite c.originalURLs on subsequent calls
+	if !c.insteadOfRulesApplied {
+		copy(c.originalURLs, c.URLs)
+	}
 
 	for i, url := range c.URLs {
 		if matchingURLRule := findLongestInsteadOfMatch(url, urlRules); matchingURLRule != nil {
 			c.URLs[i] = matchingURLRule.ApplyInsteadOf(c.URLs[i])
 			c.insteadOfRulesApplied = true
 		}
 	}
-
-	if c.insteadOfRulesApplied {
-		c.originalURLs = originalURLs
-	}
 }

options.go

@@ -87,6 +87,8 @@ type CloneOptions struct {
 	//
 	// [Reference]: https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---shared
 	Shared bool
+	// URLRules are used to substitute urls details via the config's insteadOf sections
+	URLRules map[string]*config.URL
 }
 
 // Validate validates the fields and sets the default values.

remote.go

@@ -105,6 +105,10 @@ func (r *Remote) PushContext(ctx context.Context, o *PushOptions) (err error) {
 		return err
 	}
 
+	if err := r.ensureURLRulesApplied(); err != nil {
+		return err
+	}
+
 	if o.RemoteName != r.c.Name {
 		return fmt.Errorf("remote names don't match: %s != %s", o.RemoteName, r.c.Name)
 	}
@@ -411,6 +415,10 @@ func (r *Remote) fetch(ctx context.Context, o *FetchOptions) (sto storer.Referen
 		o.RefSpecs = r.c.Fetch
 	}
 
+	if err := r.ensureURLRulesApplied(); err != nil {
+		return nil, err
+	}
+
 	if o.RemoteURL == "" {
 		o.RemoteURL = r.c.URLs[0]
 	}
@@ -1302,6 +1310,10 @@ func (r *Remote) list(ctx context.Context, o *ListOptions) (rfs []*plumbing.Refe
 		return nil, ErrEmptyUrls
 	}
 
+	if err := r.ensureURLRulesApplied(); err != nil {
+		return nil, err
+	}
+
 	s, err := newUploadPackSession(r.c.URLs[0], o.Auth, o.InsecureSkipTLS, o.CABundle, o.ProxyOptions)
 	if err != nil {
 		return nil, err
@@ -1480,3 +1492,16 @@ func (r *Remote) checkRequireRemoteRefs(requires []config.RefSpec, remoteRefs st
 	}
 	return nil
 }
+
+func (r *Remote) ensureURLRulesApplied() error {
+	if r.s != nil {
+		cfg, err := r.s.Config()
+		if err != nil {
+			return err
+		}
+
+		r.c.ApplyURLRules(cfg.URLs)
+	}
+
+	return nil
+}

remote_test.go

@@ -562,7 +562,6 @@ func (s *RemoteSuite) TestPushToEmptyRepository(c *C) {
 	c.Assert(err, IsNil)
 
 	AssertReferences(c, server, expected)
-
 }
 
 func (s *RemoteSuite) TestPushContext(c *C) {
@@ -1555,3 +1554,102 @@ func (s *RemoteSuite) TestFetchAfterShallowClone(c *C) {
 		plumbing.NewSymbolicReference("HEAD", "refs/heads/master"),
 	})
 }
+
+// ensure insteadOf substitution is applied on Fetch()
+func (s *RemoteSuite) TestFetchInsteadOf(c *C) {
+	url := s.GetLocalRepositoryURL(fixtures.ByTag("tags").One())
+	prefix := url[:len(url)-1]
+	suffix := url[len(url)-1:]
+
+	// modify url to be invalid if insteadof is not applied
+	url = "tobereplaced" + suffix
+
+	r := NewRemote(memory.NewStorage(), &config.RemoteConfig{
+		URLs: []string{url},
+	})
+
+	cfg, err := r.s.Config()
+	c.Assert(err, IsNil)
+
+	cfg.URLs[prefix] = &config.URL{
+		Name:      prefix,
+		InsteadOf: "tobereplaced",
+	}
+
+	c.Assert(r.s.SetConfig(cfg), IsNil)
+
+	s.testFetch(c, r, &FetchOptions{
+		RefSpecs: []config.RefSpec{
+			config.RefSpec("+refs/heads/master:refs/remotes/origin/master"),
+		},
+	}, []*plumbing.Reference{
+		plumbing.NewReferenceFromStrings("refs/remotes/origin/master", "f7b877701fbf855b44c0a9e86f3fdce2c298b07f"),
+	})
+}
+
+// ensure insteadOf substitution is applied on Push()
+//
+// adapted from TestPushNoErrAlreadyUpToDate(...)
+func (s *RemoteSuite) TestPushInsteadOf(c *C) {
+	fs := fixtures.Basic().One().DotGit()
+	sto := filesystem.NewStorage(fs, cache.NewObjectLRUDefault())
+
+	url := fs.Root()
+	prefix := url[:len(url)-1]
+	suffix := url[len(url)-1:]
+
+	// modify url to be invalid if insteadof is not applied
+	url = "tobereplaced" + suffix
+
+	r := NewRemote(sto, &config.RemoteConfig{
+		Name: DefaultRemoteName,
+		URLs: []string{url},
+	})
+
+	cfg, err := r.s.Config()
+	c.Assert(err, IsNil)
+
+	cfg.URLs[prefix] = &config.URL{
+		Name:      prefix,
+		InsteadOf: "tobereplaced",
+	}
+
+	c.Assert(r.s.SetConfig(cfg), IsNil)
+
+	err = r.Push(&PushOptions{
+		RefSpecs: []config.RefSpec{"refs/heads/*:refs/heads/*"},
+	})
+	c.Assert(err, Equals, NoErrAlreadyUpToDate)
+}
+
+// ensure insteadOf substitution is applied on List()
+//
+// adapted from TestList(...)
+func (s *RemoteSuite) TestListInsteadOf(c *C) {
+	repo := fixtures.Basic().One()
+
+	url := repo.URL
+	prefix := url[:len(url)-1]
+	suffix := url[len(url)-1:]
+
+	// modify url to be invalid if insteadof is not applied
+	url = "tobereplaced" + suffix
+
+	r := NewRemote(memory.NewStorage(), &config.RemoteConfig{
+		Name: DefaultRemoteName,
+		URLs: []string{url},
+	})
+
+	cfg, err := r.s.Config()
+	c.Assert(err, IsNil)
+
+	cfg.URLs[prefix] = &config.URL{
+		Name:      prefix,
+		InsteadOf: "tobereplaced",
+	}
+
+	c.Assert(r.s.SetConfig(cfg), IsNil)
+
+	_, err = r.List(&ListOptions{})
+	c.Assert(err, IsNil)
+}