diff --git a/cmd/gitserver/internal/vcssyncer/perforce.go b/cmd/gitserver/internal/vcssyncer/perforce.go index dbf0034d94b..10a720caa1a 100644 --- 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 } diff --git a/dev/gqltest/external_service_test.go b/dev/gqltest/external_service_test.go index aca2ff6b6f6..ecd3dac330a 100644 --- 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{ diff --git a/dev/gqltest/sub_repo_permissions_test.go b/dev/gqltest/sub_repo_permissions_test.go index f2f412aa14d..594508b82da 100644 --- 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 { diff --git a/schema/perforce.schema.json b/schema/perforce.schema.json index ecc3845e585..7e66bf20a20 100644 --- 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 } } } diff --git a/schema/schema.go b/schema/schema.go index aeb2bc7b464..28c9c4e1d2e 100644 --- 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