Merge pull request #5309 from bk2204/ssh-transfer-manifest

Avoid needlessly spawning SSH connections with `git archive`

commands/commands.go

@@ -34,7 +34,7 @@ var (
 	ErrorWriter  = newMultiWriter(os.Stderr, ErrorBuffer)
 	OutputWriter = newMultiWriter(os.Stdout, ErrorBuffer)
 	ManPages     = make(map[string]string, 20)
-	tqManifest   = make(map[string]*tq.Manifest)
+	tqManifest   = make(map[string]tq.Manifest)
 
 	cfg       *config.Configuration
 	apiClient *lfsapi.Client
@@ -48,14 +48,14 @@ var (
 
 // getTransferManifest builds a tq.Manifest from the global os and git
 // environments.
-func getTransferManifest() *tq.Manifest {
+func getTransferManifest() tq.Manifest {
 	return getTransferManifestOperationRemote("", "")
 }
 
 // getTransferManifestOperationRemote builds a tq.Manifest from the global os
 // and git environments and operation-specific and remote-specific settings.
 // Operation must be "download", "upload", or the empty string.
-func getTransferManifestOperationRemote(operation, remote string) *tq.Manifest {
+func getTransferManifestOperationRemote(operation, remote string) tq.Manifest {
 	c := getAPIClient()
 
 	global.Lock()
@@ -112,14 +112,14 @@ func newLockClient() *locking.Client {
 }
 
 // newDownloadCheckQueue builds a checking queue, checks that objects are there but doesn't download
-func newDownloadCheckQueue(manifest *tq.Manifest, remote string, options ...tq.Option) *tq.TransferQueue {
+func newDownloadCheckQueue(manifest tq.Manifest, remote string, options ...tq.Option) *tq.TransferQueue {
 	return newDownloadQueue(manifest, remote, append(options,
 		tq.DryRun(true),
 	)...)
 }
 
 // newDownloadQueue builds a DownloadQueue, allowing concurrent downloads.
-func newDownloadQueue(manifest *tq.Manifest, remote string, options ...tq.Option) *tq.TransferQueue {
+func newDownloadQueue(manifest tq.Manifest, remote string, options ...tq.Option) *tq.TransferQueue {
 	return tq.NewTransferQueue(tq.Download, manifest, remote, append(options,
 		tq.RemoteRef(currentRemoteRef()),
 	)...)

commands/lockverifier.go

@@ -151,7 +151,7 @@ func (lv *lockVerifier) newRefLocks(ref *git.Ref, l locking.Lock) *refLock {
 	}
 }
 
-func newLockVerifier(m *tq.Manifest) *lockVerifier {
+func newLockVerifier(m tq.Manifest) *lockVerifier {
 	lv := &lockVerifier{
 		endpoint:     getAPIClient().Endpoints.Endpoint("upload", cfg.PushRemote()),
 		verifiedRefs: make(map[string]bool),

commands/pull.go

@@ -39,7 +39,7 @@ func newSingleCheckout(gitEnv config.Environment, remote string) abstractCheckou
 }
 
 type abstractCheckout interface {
-	Manifest() *tq.Manifest
+	Manifest() tq.Manifest
 	Skip() bool
 	Run(*lfs.WrappedPointer)
 	RunToPath(*lfs.WrappedPointer, string) error
@@ -49,11 +49,11 @@ type abstractCheckout interface {
 type singleCheckout struct {
 	gitIndexer    *gitIndexer
 	pathConverter lfs.PathConverter
-	manifest      *tq.Manifest
+	manifest      tq.Manifest
 	remote        string
 }
 
-func (c *singleCheckout) Manifest() *tq.Manifest {
+func (c *singleCheckout) Manifest() tq.Manifest {
 	if c.manifest == nil {
 		c.manifest = getTransferManifestOperationRemote("download", c.remote)
 	}
@@ -115,11 +115,11 @@ func (c *singleCheckout) Close() {
 }
 
 type noOpCheckout struct {
-	manifest *tq.Manifest
+	manifest tq.Manifest
 	remote   string
 }
 
-func (c *noOpCheckout) Manifest() *tq.Manifest {
+func (c *noOpCheckout) Manifest() tq.Manifest {
 	if c.manifest == nil {
 		c.manifest = getTransferManifestOperationRemote("download", c.remote)
 	}

commands/uploader.go

@@ -72,7 +72,7 @@ func uploadRangeOrAll(g *lfs.GitScanner, ctx *uploadContext, q *tq.TransferQueue
 type uploadContext struct {
 	Remote       string
 	DryRun       bool
-	Manifest     *tq.Manifest
+	Manifest     tq.Manifest
 	uploadedOids tools.StringSet
 	gitfilter    *lfs.GitFilter
 

lfs/gitfilter_smudge.go

@@ -15,7 +15,7 @@ import (
 	"github.com/rubyist/tracerx"
 )
 
-func (f *GitFilter) SmudgeToFile(filename string, ptr *Pointer, download bool, manifest *tq.Manifest, cb tools.CopyCallback) error {
+func (f *GitFilter) SmudgeToFile(filename string, ptr *Pointer, download bool, manifest tq.Manifest, cb tools.CopyCallback) error {
 	tools.MkdirAll(filepath.Dir(filename), f.cfg)
 
 	if stat, _ := os.Stat(filename); stat != nil && stat.Mode()&0200 == 0 {
@@ -52,7 +52,7 @@ func (f *GitFilter) SmudgeToFile(filename string, ptr *Pointer, download bool, m
 	return nil
 }
 
-func (f *GitFilter) Smudge(writer io.Writer, ptr *Pointer, workingfile string, download bool, manifest *tq.Manifest, cb tools.CopyCallback) (int64, error) {
+func (f *GitFilter) Smudge(writer io.Writer, ptr *Pointer, workingfile string, download bool, manifest tq.Manifest, cb tools.CopyCallback) (int64, error) {
 	mediafile, err := f.ObjectPath(ptr.Oid)
 	if err != nil {
 		return 0, err
@@ -99,7 +99,7 @@ func (f *GitFilter) Smudge(writer io.Writer, ptr *Pointer, workingfile string, d
 	return n, nil
 }
 
-func (f *GitFilter) downloadFile(writer io.Writer, ptr *Pointer, workingfile, mediafile string, manifest *tq.Manifest, cb tools.CopyCallback) (int64, error) {
+func (f *GitFilter) downloadFile(writer io.Writer, ptr *Pointer, workingfile, mediafile string, manifest tq.Manifest, cb tools.CopyCallback) (int64, error) {
 	fmt.Fprintln(os.Stderr, tr.Tr.Get("Downloading %s (%s)", workingfile, humanize.FormatBytes(uint64(ptr.Size))))
 
 	// NOTE: if given, "cb" is a tools.CopyCallback which writes updates
@@ -131,7 +131,7 @@ func (f *GitFilter) downloadFile(writer io.Writer, ptr *Pointer, workingfile, me
 	return f.readLocalFile(writer, ptr, mediafile, workingfile, nil)
 }
 
-func (f *GitFilter) downloadFileFallBack(writer io.Writer, ptr *Pointer, workingfile, mediafile string, manifest *tq.Manifest, cb tools.CopyCallback) (int64, error) {
+func (f *GitFilter) downloadFileFallBack(writer io.Writer, ptr *Pointer, workingfile, mediafile string, manifest tq.Manifest, cb tools.CopyCallback) (int64, error) {
 	// Attempt to find the LFS objects in all currently registered remotes.
 	// When a valid remote is found, this remote is taken persistent for
 	// future attempts within downloadFile(). In best case, the ordinary

lfs/lfs.go

@@ -15,7 +15,7 @@ import (
 	"github.com/rubyist/tracerx"
 )
 
-func Environ(cfg *config.Configuration, manifest *tq.Manifest, envOverrides map[string]string) []string {
+func Environ(cfg *config.Configuration, manifest tq.Manifest, envOverrides map[string]string) []string {
 	osEnviron := os.Environ()
 	env := make([]string, 0, len(osEnviron)+7)
 

t/git-lfs-test-server-api/main.go

@@ -27,7 +27,7 @@ type TestObject struct {
 
 type ServerTest struct {
 	Name string
-	F    func(m *tq.Manifest, oidsExist, oidsMissing []TestObject) error
+	F    func(m tq.Manifest, oidsExist, oidsMissing []TestObject) error
 }
 
 var (
@@ -138,7 +138,7 @@ func (*testDataCallback) Errorf(format string, args ...interface{}) {
 	fmt.Printf(format, args...)
 }
 
-func buildManifest(r *t.Repo) (*tq.Manifest, error) {
+func buildManifest(r *t.Repo) (tq.Manifest, error) {
 	// Configure the endpoint manually
 	finder := lfsapi.NewEndpointFinder(r)
 
@@ -176,7 +176,7 @@ func (c *constantEndpoint) Endpoint(operation, remote string) lfshttp.Endpoint {
 
 func (c *constantEndpoint) RemoteEndpoint(operation, remote string) lfshttp.Endpoint { return c.e }
 
-func buildTestData(repo *t.Repo, manifest *tq.Manifest) (oidsExist, oidsMissing []TestObject, err error) {
+func buildTestData(repo *t.Repo, manifest tq.Manifest) (oidsExist, oidsMissing []TestObject, err error) {
 	const oidCount = 50
 	oidsExist = make([]TestObject, 0, oidCount)
 	oidsMissing = make([]TestObject, 0, oidCount)
@@ -242,7 +242,7 @@ func saveTestOids(filename string, objs []TestObject) {
 
 }
 
-func runTests(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) bool {
+func runTests(manifest tq.Manifest, oidsExist, oidsMissing []TestObject) bool {
 	ok := true
 	fmt.Printf("Running %d tests...\n", len(tests))
 	for _, t := range tests {
@@ -254,7 +254,7 @@ func runTests(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) bool {
 	return ok
 }
 
-func runTest(t ServerTest, manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) error {
+func runTest(t ServerTest, manifest tq.Manifest, oidsExist, oidsMissing []TestObject) error {
 	const linelen = 70
 	line := t.Name
 	if len(line) > linelen {
@@ -280,11 +280,11 @@ func exit(format string, args ...interface{}) {
 	os.Exit(2)
 }
 
-func addTest(name string, f func(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) error) {
+func addTest(name string, f func(manifest tq.Manifest, oidsExist, oidsMissing []TestObject) error) {
 	tests = append(tests, ServerTest{Name: name, F: f})
 }
 
-func callBatchApi(manifest *tq.Manifest, dir tq.Direction, objs []TestObject) ([]*tq.Transfer, error) {
+func callBatchApi(manifest tq.Manifest, dir tq.Direction, objs []TestObject) ([]*tq.Transfer, error) {
 	apiobjs := make([]*tq.Transfer, 0, len(objs))
 	for _, o := range objs {
 		apiobjs = append(apiobjs, &tq.Transfer{Oid: o.Oid, Size: o.Size})

t/git-lfs-test-server-api/testdownload.go

@@ -10,7 +10,7 @@ import (
 )
 
 // "download" - all present
-func downloadAllExist(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) error {
+func downloadAllExist(manifest tq.Manifest, oidsExist, oidsMissing []TestObject) error {
 	retobjs, err := callBatchApi(manifest, tq.Download, oidsExist)
 
 	if err != nil {
@@ -37,7 +37,7 @@ func downloadAllExist(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject
 }
 
 // "download" - all missing (test includes 404 error entry)
-func downloadAllMissing(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) error {
+func downloadAllMissing(manifest tq.Manifest, oidsExist, oidsMissing []TestObject) error {
 	retobjs, err := callBatchApi(manifest, tq.Download, oidsMissing)
 
 	if err != nil {
@@ -69,7 +69,7 @@ func downloadAllMissing(manifest *tq.Manifest, oidsExist, oidsMissing []TestObje
 }
 
 // "download" - mixture
-func downloadMixed(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) error {
+func downloadMixed(manifest tq.Manifest, oidsExist, oidsMissing []TestObject) error {
 	existSet := tools.NewStringSetWithCapacity(len(oidsExist))
 	for _, o := range oidsExist {
 		existSet.Add(o.Oid)

t/git-lfs-test-server-api/testupload.go

@@ -10,7 +10,7 @@ import (
 )
 
 // "upload" - all missing
-func uploadAllMissing(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) error {
+func uploadAllMissing(manifest tq.Manifest, oidsExist, oidsMissing []TestObject) error {
 	retobjs, err := callBatchApi(manifest, tq.Upload, oidsMissing)
 
 	if err != nil {
@@ -38,7 +38,7 @@ func uploadAllMissing(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject
 }
 
 // "upload" - all present
-func uploadAllExists(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) error {
+func uploadAllExists(manifest tq.Manifest, oidsExist, oidsMissing []TestObject) error {
 	retobjs, err := callBatchApi(manifest, tq.Upload, oidsExist)
 
 	if err != nil {
@@ -65,7 +65,7 @@ func uploadAllExists(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject)
 }
 
 // "upload" - mix of missing & present
-func uploadMixed(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) error {
+func uploadMixed(manifest tq.Manifest, oidsExist, oidsMissing []TestObject) error {
 	existSet := tools.NewStringSetWithCapacity(len(oidsExist))
 	for _, o := range oidsExist {
 		existSet.Add(o.Oid)
@@ -109,7 +109,7 @@ func uploadMixed(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) err
 
 }
 
-func uploadEdgeCases(manifest *tq.Manifest, oidsExist, oidsMissing []TestObject) error {
+func uploadEdgeCases(manifest tq.Manifest, oidsExist, oidsMissing []TestObject) error {
 	errorCases := make([]TestObject, 0, 5)
 	errorCodeMap := make(map[string]int, 5)
 	errorReasonMap := make(map[string]string, 5)

t/t-filter-process.sh

@@ -241,3 +241,33 @@ begin_test "filter process: checking out a branch with --skip-smudge and checkou
   assert_pointer "b" "b.dat" "$contents_b_oid" 10
 )
 end_test
+
+begin_test "filter process: git archive does not invoke SSH"
+(
+  set -e
+
+  setup_pure_ssh
+
+  reponame="filter-process-archive"
+  setup_remote_repo "$reponame"
+  clone_repo "$reponame" "$reponame"
+
+  sshurl=$(ssh_remote "$reponame")
+  git config lfs.url "$sshurl"
+
+  contents="test"
+  git lfs track "*.dat"
+  printf "%s" "$contents" > test.dat
+  git add .gitattributes test.dat
+  git commit -m "initial commit"
+
+  git push origin main 2>&1
+  cd ..
+  GIT_TRACE=1 git clone "$sshurl" "$reponame-2" 2>&1 | tee trace.log
+  grep "lfs-ssh-echo.*git-lfs-transfer .*$reponame.git download" trace.log
+  cd "$reponame-2"
+  GIT_TRACE=1 GIT_TRACE_PACKET=1 git archive -o foo.tar HEAD 2>&1 | tee archive.log
+  grep 'pure SSH' archive.log && exit 1
+  true
+)
+end_test

tq/api.go

@@ -35,12 +35,14 @@ type BatchResponse struct {
 	endpoint            lfshttp.Endpoint
 }
 
-func Batch(m *Manifest, dir Direction, remote string, remoteRef *git.Ref, objects []*Transfer) (*BatchResponse, error) {
+func Batch(m Manifest, dir Direction, remote string, remoteRef *git.Ref, objects []*Transfer) (*BatchResponse, error) {
 	if len(objects) == 0 {
 		return &BatchResponse{}, nil
 	}
 
-	return m.batchClient().Batch(remote, &batchRequest{
+	cm := m.Upgrade()
+
+	return cm.batchClient().Batch(remote, &batchRequest{
 		Operation:            dir.String(),
 		Objects:              objects,
 		TransferAdapterNames: m.GetAdapterNames(dir),

tq/basic_download.go

@@ -261,7 +261,7 @@ func (a *basicDownloadAdapter) download(t *Transfer, cb ProgressCallback, authOk
 	return err
 }
 
-func configureBasicDownloadAdapter(m *Manifest) {
+func configureBasicDownloadAdapter(m *concreteManifest) {
 	m.RegisterNewAdapterFunc(BasicAdapterName, Download, func(name string, dir Direction) Adapter {
 		switch dir {
 		case Download:

tq/basic_upload.go

@@ -209,7 +209,7 @@ func newStartCallbackReader(r lfsapi.ReadSeekCloser, cb func() error) *startCall
 	}
 }
 
-func configureBasicUploadAdapter(m *Manifest) {
+func configureBasicUploadAdapter(m *concreteManifest) {
 	m.RegisterNewAdapterFunc(BasicAdapterName, Upload, func(name string, dir Direction) Adapter {
 		switch dir {
 		case Upload:

tq/custom.go

@@ -351,7 +351,7 @@ const (
 	standaloneFileName = "lfs-standalone-file"
 )
 
-func configureDefaultCustomAdapters(git Env, m *Manifest) {
+func configureDefaultCustomAdapters(git Env, m *concreteManifest) {
 	newfunc := func(name string, dir Direction) Adapter {
 		standalone := m.standaloneTransferAgent != ""
 		return newCustomAdapter(m.fs, standaloneFileName, dir, "git-lfs", "standalone-file", false, standalone)
@@ -361,7 +361,7 @@ func configureDefaultCustomAdapters(git Env, m *Manifest) {
 }
 
 // Initialise custom adapters based on current config
-func configureCustomAdapters(git Env, m *Manifest) {
+func configureCustomAdapters(git Env, m *concreteManifest) {
 	configureDefaultCustomAdapters(git, m)
 
 	pathRegex := regexp.MustCompile(`lfs.customtransfer.([^.]+).path`)