Enable p4-fusion by default for Perforce code host connections (#64101)

2026-02-06 15:12:02 +00:00 · 2024-08-02 11:15:16 +02:00 · 2024-08-02 11:15:16 +02:00 · 44e848d4ba
commit 44e848d4ba
parent e4ebecf205
5 changed files with 25 additions and 66 deletions
--- a/cmd/gitserver/internal/vcssyncer/perforce.go
+++ b/cmd/gitserver/internal/vcssyncer/perforce.go
@ -121,16 +121,8 @@ func (s *perforceDepotSyncer) Clone(ctx context.Context, repo api.RepoName, _ co
 	tryWrite(s.logger, progressWriter, "Perforce server connection succeeded\n")

 	var cmd *exec.Cmd
-	if s.fusionConfig.Enabled {
-		tryWrite(s.logger, progressWriter, "Converting depot using p4-fusion\n")
-		cmd = s.buildP4FusionCmd(ctx, depot, p4user, tmpPath, p4port)
-	} else {
-		tryWrite(s.logger, progressWriter, "Converting depot using git-p4\n")
-		// Example: git p4 clone --bare --max-changes 1000 //Sourcegraph/@all /tmp/clone-584194180/.git
-		args := append([]string{"p4", "clone", "--bare"}, s.p4CommandOptions()...)
-		args = append(args, depot+"@all", tmpPath)
-		cmd = exec.CommandContext(ctx, "git", args...)
-	}
+	tryWrite(s.logger, progressWriter, "Converting depot using p4-fusion\n")
+	cmd = s.buildP4FusionCmd(ctx, depot, p4user, tmpPath, p4port)
 	cmd.Env, err = s.p4CommandEnv(tmpPath, p4port, p4user, p4passwd)
 	if err != nil {
 		return errors.Wrap(err, "failed to build p4 command env")
@ -200,6 +192,7 @@ func (s *perforceDepotSyncer) buildP4FusionCmd(ctx context.Context, depot, usern
 		"--maxChanges", strconv.Itoa(s.fusionConfig.MaxChanges),
 		"--includeBinaries", strconv.FormatBool(s.fusionConfig.IncludeBinaries),
 		"--fsyncEnable", strconv.FormatBool(s.fusionConfig.FsyncEnable),
+		"--noConvertLabels", strconv.FormatBool(s.fusionConfig.NoConvertLabels),
 		"--noColor", "true",
 		// We don't want an empty commit for a sane merge base across branches,
 		// since we don't use them and the empty commit breaks changelist parsing.
@ -234,16 +227,9 @@ func (s *perforceDepotSyncer) Fetch(ctx context.Context, repoName api.RepoName,
 		return errors.Wrap(err, "verifying connection to perforce server")
 	}

-	var cmd *wrexec.Cmd
-	if s.fusionConfig.Enabled {
-		// Example: p4-fusion --path //depot/... --user $P4USER --src clones/ --networkThreads 64 --printBatch 10 --port $P4PORT --lookAhead 2000 --retries 10 --refresh 100
-		root, _ := filepath.Split(string(dir))
-		cmd = wrexec.Wrap(ctx, nil, s.buildP4FusionCmd(ctx, depot, p4user, root+".git", p4port))
-	} else {
-		// Example: git p4 sync --max-changes 1000
-		args := append([]string{"p4", "sync"}, s.p4CommandOptions()...)
-		cmd = wrexec.CommandContext(ctx, nil, "git", args...)
-	}
+	// Example: p4-fusion --path //depot/... --user $P4USER --src clones/ --networkThreads 64 --printBatch 10 --port $P4PORT --lookAhead 2000 --retries 10 --refresh 100
+	root, _ := filepath.Split(string(dir))
+	cmd := wrexec.Wrap(ctx, nil, s.buildP4FusionCmd(ctx, depot, p4user, root+".git", p4port))
 	cmd.Env, err = s.p4CommandEnv(string(dir), p4port, p4user, p4passwd)
 	if err != nil {
 		return errors.Wrap(err, "failed to build p4 command env")
@ -259,26 +245,6 @@ func (s *perforceDepotSyncer) Fetch(ctx context.Context, repoName api.RepoName,
 		return errors.Wrapf(err, "failed to update")
 	}

-	if !s.fusionConfig.Enabled {
-		p4home, err := s.fs.P4HomeDir()
-		if err != nil {
-			return errors.Wrap(err, "failed to create p4home")
-		}
-
-		// Force update "master" to "refs/remotes/p4/master" where changes are synced into
-		cmd = wrexec.CommandContext(ctx, nil, "git", "branch", "-f", "master", "refs/remotes/p4/master")
-		cmd.Cmd.Env = append(os.Environ(),
-			"P4PORT="+p4port,
-			"P4USER="+p4user,
-			"P4PASSWD="+p4passwd,
-			"HOME="+p4home,
-		)
-		dir.Set(cmd.Cmd)
-		if output, err := cmd.CombinedOutput(); err != nil {
-			return errors.Wrapf(err, "failed to force update branch with output %q", string(output))
-		}
-	}
-
 	// The update was successful, after a git fetch it is expected that a repos
 	// FETCH_HEAD has either been updated, or that HEAD has been touched, even
 	// if no changes were fetched.
@ -328,8 +294,6 @@ func (s *perforceDepotSyncer) p4CommandEnv(cmdCWD, p4port, p4user, p4passwd stri

 // fusionConfig allows configuration of the p4-fusion client.
 type fusionConfig struct {
-	// Enabled: Enable the p4-fusion client for cloning and fetching repos
-	Enabled bool
 	// Client: The client spec tht should be used
 	Client string
 	// LookAhead: How many CLs in the future, at most, shall we keep downloaded by
@ -357,12 +321,13 @@ type fusionConfig struct {
 	// written to permanent storage immediately instead of being cached. This is to
 	// mitigate data loss in events of hardware failure.
 	FsyncEnable bool
+	// NoConvertLabels disables the conversion of Perforce labels to git tags.
+	NoConvertLabels bool
 }

 func configureFusionClient(conn *schema.PerforceConnection) fusionConfig {
 	// Set up default settings first
 	fc := fusionConfig{
-		Enabled:             false,
 		Client:              conn.P4Client,
 		LookAhead:           2000,
 		NetworkThreads:      12,
@ -373,15 +338,13 @@ func configureFusionClient(conn *schema.PerforceConnection) fusionConfig {
 		MaxChanges:          -1,
 		IncludeBinaries:     false,
 		FsyncEnable:         false,
+		NoConvertLabels:     false,
 	}

 	if conn.FusionClient == nil {
 		return fc
 	}

-	// Required
-	fc.Enabled = conn.FusionClient.Enabled
-
 	// Optional
 	if conn.FusionClient.LookAhead > 0 {
 		fc.LookAhead = conn.FusionClient.LookAhead
@ -406,6 +369,7 @@ func configureFusionClient(conn *schema.PerforceConnection) fusionConfig {
 	}
 	fc.IncludeBinaries = conn.FusionClient.IncludeBinaries
 	fc.FsyncEnable = conn.FusionClient.FsyncEnable
+	fc.NoConvertLabels = conn.FusionClient.NoConvertLabels

 	return fc
 }
--- a/dev/gqltest/external_service_test.go
+++ b/dev/gqltest/external_service_test.go
@ -173,26 +173,16 @@ func TestExternalService_Perforce(t *testing.T) {
 		{
 			name:       "p4 fusion",
 			depot:      "test-perms",
-			useFusion:  true,
 			blobPath:   "README.md",
 			headBranch: "main",
 			wantBlob: `This depot is used to test user and group permissions.
-`,
-		},
-		{
-			name:       "git p4",
-			depot:      "integration-test-depot",
-			useFusion:  false,
-			blobPath:   "path.txt",
-			headBranch: "master",
-			wantBlob: `./
 `,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			repoName := "perforce/" + tc.depot
 			checkPerforceEnvironment(t)
-			cleanup := createPerforceExternalService(t, tc.depot, tc.useFusion)
+			cleanup := createPerforceExternalService(t, tc.depot)
 			t.Cleanup(cleanup)

 			err := client.WaitForReposToBeCloned(repoName)
@ -219,7 +209,7 @@ func checkPerforceEnvironment(t *testing.T) {
 // createPerforceExternalService creates an Perforce external service that
 // includes the supplied depot. It returns a function to cleanup after the test
 // which will delete the depot from disk and remove the external service.
-func createPerforceExternalService(t *testing.T, depot string, useP4Fusion bool) func() {
+func createPerforceExternalService(t *testing.T, depot string) func() {
 	t.Helper()

 	type Authorization = struct {
@ -250,7 +240,6 @@ func createPerforceExternalService(t *testing.T, depot string, useP4Fusion bool)
 			Depots:                []string{"//" + depot + "/"},
 			RepositoryPathPattern: "perforce/{depot}",
 			FusionClient: FusionClient{
-				Enabled:   useP4Fusion,
 				LookAhead: 2000,
 			},
 			Authorization: Authorization{
--- a/dev/gqltest/sub_repo_permissions_test.go
+++ b/dev/gqltest/sub_repo_permissions_test.go
@ -24,7 +24,7 @@ const (
 func TestSubRepoPermissionsPerforce(t *testing.T) {
 	checkPerforceEnvironment(t)
 	enableSubRepoPermissions(t)
-	cleanup := createPerforceExternalService(t, testPermsDepot, true)
+	cleanup := createPerforceExternalService(t, testPermsDepot)
 	t.Cleanup(cleanup)
 	userClient, repoName, err := createTestUserAndWaitForRepo(t)
 	if err != nil {
@ -84,7 +84,7 @@ func TestSubRepoPermissionsPerforce(t *testing.T) {
 func TestSubRepoPermissionsSymbols(t *testing.T) {
 	checkPerforceEnvironment(t)
 	enableSubRepoPermissions(t)
-	cleanup := createPerforceExternalService(t, testPermsDepot, true)
+	cleanup := createPerforceExternalService(t, testPermsDepot)
 	t.Cleanup(cleanup)
 	userClient, repoName, err := createTestUserAndWaitForRepo(t)
 	if err != nil {
@ -122,7 +122,7 @@ func TestSubRepoPermissionsSearch(t *testing.T) {
 	checkPerforceEnvironment(t)
 	enableSubRepoPermissions(t)
 	enableStructuralSearch(t)
-	cleanup := createPerforceExternalService(t, testPermsDepot, true)
+	cleanup := createPerforceExternalService(t, testPermsDepot)
 	t.Cleanup(cleanup)
 	userClient, _, err := createTestUserAndWaitForRepo(t)
 	if err != nil {
--- a/schema/perforce.schema.json
+++ b/schema/perforce.schema.json
@ -67,10 +67,9 @@
      "type": "object",
      "description": "Configuration for the experimental p4-fusion client",
      "additionalProperties": false,
-      "required": ["enabled"],
      "properties": {
        "enabled": {
-          "description": "Enable the p4-fusion client for cloning and fetching repos",
+          "description": "DEPRECATED. p4-fusion is always enabled.",
          "type": "boolean",
          "default": false
        },
@ -124,6 +123,11 @@
          "description": " Enable fsync() while writing objects to disk to ensure they get written to permanent storage immediately instead of being cached. This is to mitigate data loss in events of hardware failure.",
          "type": "boolean",
          "default": false
+        },
+        "noConvertLabels": {
+          "description": "Disable Perforce label to git tag conversion.",
+          "type": "boolean",
+          "default": false
        }
      }
    }
--- a/schema/schema.go
+++ b/schema/schema.go
@ -1292,8 +1292,8 @@ type FileFilters struct {

 // FusionClient description: Configuration for the experimental p4-fusion client
 type FusionClient struct {
-	// Enabled description: Enable the p4-fusion client for cloning and fetching repos
-	Enabled bool `json:"enabled"`
+	// Enabled description: DEPRECATED. p4-fusion is always enabled.
+	Enabled bool `json:"enabled,omitempty"`
 	// FsyncEnable description:  Enable fsync() while writing objects to disk to ensure they get written to permanent storage immediately instead of being cached. This is to mitigate data loss in events of hardware failure.
 	FsyncEnable bool `json:"fsyncEnable,omitempty"`
 	// IncludeBinaries description: Whether to include binary files
@ -1306,6 +1306,8 @@ type FusionClient struct {
 	NetworkThreads int `json:"networkThreads,omitempty"`
 	// NetworkThreadsFetch description: The number of threads in the threadpool for running network calls when performing fetches. Defaults to the number of logical CPUs.
 	NetworkThreadsFetch int `json:"networkThreadsFetch,omitempty"`
+	// NoConvertLabels description: Disable Perforce label to git tag conversion.
+	NoConvertLabels bool `json:"noConvertLabels,omitempty"`
 	// PrintBatch description: The p4 print batch size
 	PrintBatch int `json:"printBatch,omitempty"`
 	// Refresh description: How many times a connection should be reused before it is refreshed