Merge pull request #1299 from github/transfer-basic-only

Add `lfs.basictransfersonly` option to disable non-basic transfer adapters

config/config.go

@@ -227,6 +227,22 @@ func (c *Configuration) ConcurrentTransfers() int {
 	return uploads
 }
 
+// BasicTransfersOnly returns whether to only allow "basic" HTTP transfers
+// Default is false, including if the lfs.basictransfersonly is invalid
+func (c *Configuration) BasicTransfersOnly() bool {
+	value, ok := c.GitConfig("lfs.basictransfersonly")
+	if !ok || len(value) == 0 {
+		return false
+	}
+
+	basicOnly, err := parseConfigBool(value)
+	if err != nil {
+		return false
+	}
+
+	return basicOnly
+}
+
 func (c *Configuration) BatchTransfer() bool {
 	value, ok := c.GitConfig("lfs.batch")
 	if !ok || len(value) == 0 {

config/config_test.go

@@ -336,6 +336,35 @@ func TestConcurrentTransfersNegativeValue(t *testing.T) {
 	assert.Equal(t, 3, n)
 }
 
+func TestBasicTransfersOnlySetValue(t *testing.T) {
+	config := &Configuration{
+		gitConfig: map[string]string{
+			"lfs.basictransfersonly": "true",
+		},
+	}
+
+	b := config.BasicTransfersOnly()
+	assert.Equal(t, true, b)
+}
+
+func TestBasicTransfersOnlyDefault(t *testing.T) {
+	config := &Configuration{}
+
+	b := config.BasicTransfersOnly()
+	assert.Equal(t, false, b)
+}
+
+func TestBasicTransfersOnlyInvalidValue(t *testing.T) {
+	config := &Configuration{
+		gitConfig: map[string]string{
+			"lfs.basictransfersonly": "wat",
+		},
+	}
+
+	b := config.BasicTransfersOnly()
+	assert.Equal(t, false, b)
+}
+
 func TestBatch(t *testing.T) {
 	tests := map[string]bool{
 		"":         true,

docs/man/git-lfs-config.5.ronn

@@ -26,6 +26,12 @@ lfs option can be scoped inside the configuration for a remote.
 
   The number of concurrent uploads/downloads. Default 3.
 
+* `lfs.basictransfersonly`
+
+  If set to true, only basic HTTP upload/download transfers will be used, 
+  ignoring any more advanced transfers that the client/server may support.
+  This is primarily to work around bugs or incompatibilities.
+
 * `lfs.batch`
 
   Whether to use the batch API instead of requesting objects individually.

lfs/lfs.go

@@ -72,6 +72,7 @@ func Environ() []string {
 		fmt.Sprintf("LocalReferenceDir=%s", config.LocalReferenceDir),
 		fmt.Sprintf("TempDir=%s", TempDir()),
 		fmt.Sprintf("ConcurrentTransfers=%d", config.Config.ConcurrentTransfers()),
+		fmt.Sprintf("BasicTransfersOnly=%v", config.Config.BasicTransfersOnly()),
 		fmt.Sprintf("BatchTransfer=%v", config.Config.BatchTransfer()),
 		fmt.Sprintf("SkipDownloadErrors=%v", config.Config.SkipDownloadErrors()),
 		fmt.Sprintf("FetchRecentAlways=%v", config.Config.FetchPruneConfig().FetchRecentAlways),

test/test-env.sh

@@ -30,6 +30,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -77,6 +78,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -131,6 +133,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -183,6 +186,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -237,6 +241,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -292,6 +297,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -349,6 +355,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=5
+BasicTransfersOnly=false
 BatchTransfer=false
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -412,6 +419,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -469,6 +477,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -519,6 +528,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -560,6 +570,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -590,6 +601,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -619,6 +631,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -659,6 +672,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -730,6 +744,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=true
 FetchRecentAlways=false
@@ -759,6 +774,7 @@ LocalMediaDir=%s
 LocalReferenceDir=
 TempDir=%s
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=true
 FetchRecentAlways=false

test/test-worktree.sh

@@ -27,6 +27,7 @@ LocalMediaDir=$(native_path_escaped "$TRASHDIR/$reponame/.git/lfs/objects")
 LocalReferenceDir=
 TempDir=$(native_path_escaped "$TRASHDIR/$reponame/.git/lfs/tmp")
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false
@@ -59,6 +60,7 @@ LocalMediaDir=$(native_path_escaped "$TRASHDIR/$reponame/.git/lfs/objects")
 LocalReferenceDir=
 TempDir=$(native_path_escaped "$TRASHDIR/$reponame/.git/worktrees/$worktreename/lfs/tmp")
 ConcurrentTransfers=3
+BasicTransfersOnly=false
 BatchTransfer=true
 SkipDownloadErrors=false
 FetchRecentAlways=false

transfer/transfer.go

@@ -5,6 +5,8 @@ package transfer
 import (
 	"sync"
 
+	"github.com/github/git-lfs/config"
+
 	"github.com/github/git-lfs/api"
 	"github.com/rubyist/tracerx"
 )
@@ -104,6 +106,11 @@ func GetAdapterNames(dir Direction) []string {
 
 // GetDownloadAdapterNames returns a list of the names of download adapters available to be created
 func GetDownloadAdapterNames() []string {
+
+	if config.Config.BasicTransfersOnly() {
+		return []string{BasicAdapterName}
+	}
+
 	funcMutex.Lock()
 	defer funcMutex.Unlock()
 
@@ -116,6 +123,11 @@ func GetDownloadAdapterNames() []string {
 
 // GetUploadAdapterNames returns a list of the names of upload adapters available to be created
 func GetUploadAdapterNames() []string {
+
+	if config.Config.BasicTransfersOnly() {
+		return []string{BasicAdapterName}
+	}
+
 	funcMutex.Lock()
 	defer funcMutex.Unlock()
 

transfer/transfer_test.go

@@ -3,6 +3,8 @@ package transfer
 import (
 	"testing"
 
+	"github.com/github/git-lfs/config"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -103,3 +105,20 @@ func testAdapterRegAndOverride(t *testing.T) {
 	}
 
 }
+
+func testAdapterRegButBasicOnly(t *testing.T) {
+	assert := assert.New(t)
+
+	config.Config.SetConfig("lfs.basictransfersonly", "yes")
+	RegisterNewTransferAdapterFunc("test", Upload, newTestAdapter)
+	RegisterNewTransferAdapterFunc("test", Download, newTestAdapter)
+	// Will still be created if we ask for them
+	assert.NotNil(NewUploadAdapter("test"))
+	assert.NotNil(NewDownloadAdapter("test"))
+
+	// But list will exclude
+	ld := GetDownloadAdapterNames()
+	assert.Equal([]string{BasicAdapterName}, ld)
+	lu := GetUploadAdapterNames()
+	assert.Equal([]string{BasicAdapterName}, lu)
+}