diff --git a/client/web-sveltekit/src/lib/repo/LastCommit.gql b/client/web-sveltekit/src/lib/repo/LastCommit.gql index 8335e6bbd58e..e088b2409f4a 100644 --- a/client/web-sveltekit/src/lib/repo/LastCommit.gql +++ b/client/web-sveltekit/src/lib/repo/LastCommit.gql @@ -7,6 +7,8 @@ fragment LastCommitFragment on GitCommit { date person { ...Avatar_Person + displayName + name } } } diff --git a/client/web-sveltekit/src/lib/repo/LastCommit.svelte b/client/web-sveltekit/src/lib/repo/LastCommit.svelte index cb9d25084fc1..0f4346f42f20 100644 --- a/client/web-sveltekit/src/lib/repo/LastCommit.svelte +++ b/client/web-sveltekit/src/lib/repo/LastCommit.svelte @@ -18,7 +18,7 @@
- {user.name} + {user.displayName || user.name}
diff --git a/client/web-sveltekit/src/lib/search/resultsIndicator/ProgressMessage.svelte b/client/web-sveltekit/src/lib/search/resultsIndicator/ProgressMessage.svelte index fd4fba374bbb..e1bd2a688a58 100644 --- a/client/web-sveltekit/src/lib/search/resultsIndicator/ProgressMessage.svelte +++ b/client/web-sveltekit/src/lib/search/resultsIndicator/ProgressMessage.svelte @@ -21,7 +21,8 @@ diff --git a/client/web-sveltekit/src/routes/svelte-overrides.scss b/client/web-sveltekit/src/routes/svelte-overrides.scss index c915a3cbcd67..aca2b31ab644 100644 --- a/client/web-sveltekit/src/routes/svelte-overrides.scss +++ b/client/web-sveltekit/src/routes/svelte-overrides.scss @@ -12,8 +12,9 @@ h6 { } body { - --font-size-small: 0.875rem; --font-size-base: 0.9375rem; + --font-size-small: 0.875rem; + --font-size-tiny: 0.8125rem; --code-font-size: 13px; --border-radius: 4px; diff --git a/cmd/frontend/graphqlbackend/external_account.go b/cmd/frontend/graphqlbackend/external_account.go index 364926d324dd..b6a3a835d593 100644 --- a/cmd/frontend/graphqlbackend/external_account.go +++ b/cmd/frontend/graphqlbackend/external_account.go @@ -2,7 +2,6 @@ package graphqlbackend import ( "context" - "fmt" "github.com/graph-gophers/graphql-go" "github.com/graph-gophers/graphql-go/relay" @@ -63,7 +62,7 @@ func (r *externalAccountResolver) CodySubscription(ctx context.Context) (*CodySu return nil, errors.New("this feature is only available on sourcegraph.com") } - if r.account.ServiceType != "openidconnect" || r.account.ServiceID != fmt.Sprintf("https://%s", ssc.GetSAMSHostName()) { + if r.account.ServiceType != "openidconnect" || r.account.ServiceID != ssc.GetSAMSServiceID() { return nil, nil } diff --git a/cmd/frontend/internal/app/ui/router.go b/cmd/frontend/internal/app/ui/router.go index e66afe0c65b7..920f47fa620b 100644 --- a/cmd/frontend/internal/app/ui/router.go +++ b/cmd/frontend/internal/app/ui/router.go @@ -80,6 +80,13 @@ var aboutRedirects = map[string]string{ "help/terms": "terms", } +type staticPageInfo struct { + // Specify either path OR pathPrefix. + path, pathPrefix string + name, title string + index bool +} + // Router returns the router that serves pages for our web app. func Router() *mux.Router { return uirouter.Router @@ -111,13 +118,8 @@ func InitRouter(db database.DB) { ghAppRouter := r.PathPrefix("/githubapp/").Subrouter() githubapp.SetupGitHubAppRoutes(ghAppRouter, db) - // Basic pages with static titles - for _, p := range []struct { - // Specify either path OR pathPrefix. - path, pathPrefix string - name, title string - index bool - }{ + // Basic pages with static titles. + staticPages := []staticPageInfo{ // with index: {pathPrefix: "/insights", name: "insights", title: "Insights", index: true}, {pathPrefix: "/search-jobs", name: "search-jobs", title: "Search Jobs", index: true}, @@ -160,7 +162,8 @@ func InitRouter(db database.DB) { {path: "/survey", name: "survey", title: "Survey", index: false}, {path: "/survey/{score}", name: "survey-score", title: "Survey", index: false}, {path: "/welcome", name: "welcome", title: "Welcome", index: false}, - } { + } + for _, p := range staticPages { var handler http.Handler if p.index { handler = brandedIndex(p.title) diff --git a/cmd/frontend/internal/cody/subscription.go b/cmd/frontend/internal/cody/subscription.go index 229f05289e09..003bb46b28ee 100644 --- a/cmd/frontend/internal/cody/subscription.go +++ b/cmd/frontend/internal/cody/subscription.go @@ -2,7 +2,6 @@ package cody import ( "context" - "fmt" "sync" "time" @@ -88,14 +87,10 @@ func consolidateSubscriptionDetails(ctx context.Context, user types.User, subscr // will return ("", nil). After we migrate all dotcom user accounts to SAMS, that // should no longer be possible. func getSAMSAccountIDsForUser(ctx context.Context, db database.DB, dotcomUserID int32) ([]string, error) { - // NOTE: We hard-code this to look for the SAMS-prod environment, meaning there isn't a way - // to test dotcom pulling subscription data from a local SAMS/SSC instance. To support that - // we'd need to make the SAMSHostname configurable. (Or somehow identify which OIDC provider - // is SAMS.) oidcAccounts, err := db.UserExternalAccounts().List(ctx, database.ExternalAccountsListOptions{ UserID: dotcomUserID, ServiceType: "openidconnect", - ServiceID: fmt.Sprintf("https://%s", ssc.GetSAMSHostName()), + ServiceID: ssc.GetSAMSServiceID(), }) if err != nil { return []string{}, errors.Wrap(err, "listing external accounts") @@ -137,7 +132,6 @@ func SubscriptionForUser(ctx context.Context, db database.DB, user types.User) ( // While developing the SSC backend, we only fetch subscription data for users based on a flag. var subscription *ssc.Subscription if samsAccountID != "" { - subscription, err = sscClient.FetchSubscriptionBySAMSAccountID(ctx, samsAccountID) if err != nil { return nil, errors.Wrap(err, "fetching subscription from SSC") diff --git a/cmd/frontend/internal/cody/subscription_test.go b/cmd/frontend/internal/cody/subscription_test.go index e95819866371..ecb77d810c4c 100644 --- a/cmd/frontend/internal/cody/subscription_test.go +++ b/cmd/frontend/internal/cody/subscription_test.go @@ -2,7 +2,6 @@ package cody import ( "context" - "fmt" "testing" "time" @@ -130,7 +129,7 @@ func TestGetSubscriptionForUser(t *testing.T) { accounts = append(accounts, &extsvc.Account{AccountSpec: extsvc.AccountSpec{ AccountID: MockSSCValue.SAMSAccountID, ServiceType: "openidconnect", - ServiceID: fmt.Sprintf("https://%s/", ssc.GetSAMSHostName()), + ServiceID: ssc.GetSAMSServiceID(), }}) } diff --git a/cmd/frontend/internal/dotcom/productsubscription/codygateway_dotcom_user_test.go b/cmd/frontend/internal/dotcom/productsubscription/codygateway_dotcom_user_test.go index 1385d21a6bb1..b14a00dae44f 100644 --- a/cmd/frontend/internal/dotcom/productsubscription/codygateway_dotcom_user_test.go +++ b/cmd/frontend/internal/dotcom/productsubscription/codygateway_dotcom_user_test.go @@ -2,7 +2,6 @@ package productsubscription_test import ( "context" - "fmt" "math" "testing" "time" @@ -313,7 +312,7 @@ func TestCodyGatewayCompletionsRateLimit(t *testing.T) { AccountSpec: extsvc.AccountSpec{ AccountID: "123", ServiceType: "openidconnect", - ServiceID: fmt.Sprintf("https://%s", ssc.GetSAMSHostName()), + ServiceID: ssc.GetSAMSServiceID(), }, }) require.NoError(t, err) @@ -328,7 +327,7 @@ func TestCodyGatewayCompletionsRateLimit(t *testing.T) { AccountSpec: extsvc.AccountSpec{ AccountID: "456", ServiceType: "openidconnect", - ServiceID: fmt.Sprintf("https://%s", ssc.GetSAMSHostName()), + ServiceID: ssc.GetSAMSServiceID(), }, }) require.NoError(t, err) @@ -343,7 +342,7 @@ func TestCodyGatewayCompletionsRateLimit(t *testing.T) { AccountSpec: extsvc.AccountSpec{ AccountID: "789", ServiceType: "openidconnect", - ServiceID: fmt.Sprintf("https://%s", ssc.GetSAMSHostName()), + ServiceID: ssc.GetSAMSServiceID(), }, }) require.NoError(t, err) @@ -358,7 +357,7 @@ func TestCodyGatewayCompletionsRateLimit(t *testing.T) { AccountSpec: extsvc.AccountSpec{ AccountID: "abc", ServiceType: "openidconnect", - ServiceID: fmt.Sprintf("https://%s", ssc.GetSAMSHostName()), + ServiceID: ssc.GetSAMSServiceID(), }, }) require.NoError(t, err) diff --git a/cmd/frontend/internal/httpapi/ssc.go b/cmd/frontend/internal/httpapi/ssc.go index 9ca4ee6de7a2..b1c55a3930e7 100644 --- a/cmd/frontend/internal/httpapi/ssc.go +++ b/cmd/frontend/internal/httpapi/ssc.go @@ -27,7 +27,7 @@ func newSSCRefreshCodyRateLimitHandler(logger log.Logger, db database.DB) http.H oidcAccounts, err := db.UserExternalAccounts().List(ctx, database.ExternalAccountsListOptions{ AccountID: samsAccountID, ServiceType: "openidconnect", - ServiceID: fmt.Sprintf("https://%s", ssc.GetSAMSHostName()), + ServiceID: ssc.GetSAMSServiceID(), LimitOffset: &database.LimitOffset{ Limit: 1, }, diff --git a/cmd/frontend/internal/ssc/ssc.go b/cmd/frontend/internal/ssc/ssc.go index abee5db55c43..8ca817752843 100644 --- a/cmd/frontend/internal/ssc/ssc.go +++ b/cmd/frontend/internal/ssc/ssc.go @@ -113,13 +113,52 @@ func (c *client) FetchSubscriptionBySAMSAccountID(ctx context.Context, samsAccou } } -func GetSAMSHostName() string { - sgconf := conf.Get().SiteConfig() +// getSSCBaseURL returns the base URL for the SSC backend's REST API for +// service-to-service requests. +func getSSCBaseURL() string { + config := conf.Get() + + // Prefer the newer "dotcom.codyProConfig.sscBackendOrigin" config setting if available. + // This allows for local development (not hard-coding the https scheme). + if dotcomConfig := config.Dotcom; dotcomConfig != nil { + if codyProConfig := dotcomConfig.CodyProConfig; codyProConfig != nil { + return fmt.Sprintf("%s/cody/api", codyProConfig.SscBackendOrigin) + } + } + + // Fall back to original logic, using the "ssc.apiBaseUrl" setting. + // (To be removed when the codyProConfig changes are in production.) + siteConfig := config.SiteConfig() + baseURL := siteConfig.SscApiBaseUrl + if baseURL == "" { + baseURL = "https://accounts.sourcegraph.com/cody/api" + } + + return baseURL +} + +// GetSAMSServiceID returns the ServiceID of the currently registered SAMS identity provider. +// This is found in the site configuration, and must match the auth.providers configuration +// exactly. +func GetSAMSServiceID() string { + config := conf.Get() + + // Prefer the newer "dotcom.codyProConfig.samsBackendOrigin" config setting if available. + // This allows for local development (not hard-coding the https scheme). + if dotcomConfig := config.Dotcom; dotcomConfig != nil { + if codyProConfig := dotcomConfig.CodyProConfig; codyProConfig != nil { + return codyProConfig.SamsBackendOrigin + } + } + + // Fallback to the original logic, using the "ssc.samsHostName" setting. + // (To be removed when the codyProConfig changes are in production.) + sgconf := config.SiteConfig() if sgconf.SscSamsHostName == "" { // If unset, default to the production hostname. - return "accounts.sourcegraph.com" + return "https://accounts.sourcegraph.com" } - return sgconf.SscSamsHostName + return fmt.Sprintf("https://%s", sgconf.SscSamsHostName) } // NewClient returns a new SSC API client. It is important to avoid creating new @@ -130,8 +169,6 @@ func GetSAMSHostName() string { // If no SAMS authorization provider is configured, this function will not panic, // but instead will return an error on every call. func NewClient() (Client, error) { - sgconf := conf.Get().SiteConfig() - // Fetch the SAMS configuration data. var samsConfig *clientcredentials.Config for _, provider := range conf.Get().AuthProviders { @@ -140,7 +177,7 @@ func NewClient() (Client, error) { continue } - if strings.Contains(oidcInfo.Issuer, GetSAMSHostName()) { + if oidcInfo.Issuer == GetSAMSServiceID() { samsConfig = &clientcredentials.Config{ ClientID: oidcInfo.ClientID, ClientSecret: oidcInfo.ClientSecret, @@ -155,17 +192,12 @@ func NewClient() (Client, error) { return &client{}, errors.New("no SAMS authorization provider configured") } - baseURL := sgconf.SscApiBaseUrl - if baseURL == "" { - baseURL = "https://accounts.sourcegraph.com/cody/api" - } - // We want this tokenSource to be long lived, so we benefit from reusing existing // SAMS tokens if repeated requests are made within the token's lifetime. (Under // the hood it returns an oauth2.ReuseTokenSource.) tokenSource := samsConfig.TokenSource(context.Background()) return &client{ - baseURL: baseURL, + baseURL: getSSCBaseURL(), samsTokenSource: tokenSource, }, nil } diff --git a/cmd/gitserver/BUILD.bazel b/cmd/gitserver/BUILD.bazel index bb195631bc17..a8362b5661c9 100644 --- a/cmd/gitserver/BUILD.bazel +++ b/cmd/gitserver/BUILD.bazel @@ -27,19 +27,6 @@ pkg_tar( srcs = [":gitserver"], ) -pkg_tar( - name = "tar_p4_fusion_wrappers", - srcs = [ - "p4-fusion-wrapper-detect-kill.sh", - "process-stats-watcher.sh", - ], - remap_paths = { - "/p4-fusion-wrapper-detect-kill.sh": "/usr/local/bin/p4-fusion", - "/process-stats-watcher.sh": "/usr/local/bin/process-stats-watcher.sh", - }, - visibility = ["//visibility:public"], -) - oci_image( name = "image", base = ":base_image", @@ -50,7 +37,6 @@ oci_image( ], tars = [ ":tar_gitserver", - ":tar_p4_fusion_wrappers", ], user = "sourcegraph", workdir = "/", diff --git a/cmd/gitserver/README.md b/cmd/gitserver/README.md index 5e03801d90ad..7296e972b9e0 100644 --- a/cmd/gitserver/README.md +++ b/cmd/gitserver/README.md @@ -52,20 +52,3 @@ To use `p4-fusion` while developing Sourcegraph, there are a couple of options. #### Bazel Native binaries are provided through Bazel, built via Nix in [our fork of p4-fusion](https://github.com/sourcegraph/p4-fusion/actions/workflows/nix-build-and-upload.yaml). It can be invoked either through `./dev/p4-fusion-dev` or directly with `bazel run //dev/tools:p4-fusion`. - -#### Native binary executable - -The `p4-fusion` native binary has been built on Linux and macOS, but is untested on Windows. - -Read the [comprehensive instructions](https://sourcegraph.com/docs/dev/background-information/build_p4_fusion). - -If you do go the native binary route, you may also want to enable using the wrapper shell script that detects when the process has been killed and outputs an error so that the calling process can handle it. - -That wrapper shell script is `p4-fusion-wrapper-detect-kill.sh`, and in order to use it: - -1. Rename the `p4-fusion` binary executable to `p4-fusion-binary` and move it to a location in the `PATH`. -1. Copy the shell script `p4-fusion-wrapper-detect-kill.sh` to a location in the `PATH`, renaming it `p4-fusion`. -1. Copy the shell script `process-stats-watcher.sh` to a location in the `PATH`. -1. Ensure all three of those are executable. - -After those steps, when a native `gitserver` process runs `p4-fusion`, it will run the wrapper shell script, which will itself run the `p4-fusion-binary` executable, and the `process-stats-watcher.sh` executable. diff --git a/cmd/gitserver/image_test.yaml b/cmd/gitserver/image_test.yaml index 67c29326732c..4c6fea8becca 100644 --- a/cmd/gitserver/image_test.yaml +++ b/cmd/gitserver/image_test.yaml @@ -38,7 +38,7 @@ commandTests: - name: "coursier is runnable" command: "coursier" - name: "p4-fusion is runnable" - command: "p4-fusion-binary" + command: "p4-fusion" args: expectedOutput: ["--noBaseCommit"] exitCode: 1 @@ -56,16 +56,3 @@ fileExistenceTests: shouldExist: true uid: 100 gid: 101 -# p4-fusion wrappers -- name: '/usr/local/bin/p4-fusion' - path: '/usr/local/bin/p4-fusion' - shouldExist: true - uid: 0 - gid: 0 - permissions: '-r-xr-xr-x' -- name: '/usr/local/bin/process-stats-watcher.sh' - path: '/usr/local/bin/process-stats-watcher.sh' - shouldExist: true - uid: 0 - gid: 0 - permissions: '-r-xr-xr-x' diff --git a/cmd/gitserver/p4-fusion-wrapper-detect-kill.sh b/cmd/gitserver/p4-fusion-wrapper-detect-kill.sh deleted file mode 100755 index edf587769e7a..000000000000 --- a/cmd/gitserver/p4-fusion-wrapper-detect-kill.sh +++ /dev/null @@ -1,75 +0,0 @@ -#!/usr/bin/env bash -# shellcheck disable=SC2064,SC2207 - -# create a file to hold the output of p4-fusion -# TODO: consider recording/storing/capturing the file for logs display in the UI if there's a problem -fusionout=$(mktemp || mktemp -t fusionout_XXXXXXXX) - -# create a pipe to use for capturing output of p4-fusion -# so that it can be sent to stdout and also to a file for analyzing later -fusionpipe=$(mktemp || mktemp -t fusionpipe_XXXXXXXX) -rm -f "${fusionpipe}" -mknod "${fusionpipe}" p -tee <"${fusionpipe}" "${fusionout}" & - -# create a file to hold the output of `wait` -waitout=$(mktemp || mktemp -t waitout_XXXXXXXX) - -# create a file to hold the resource usage of the child process -stats=$(mktemp || mktemp -t resource_XXXXXXXX) - -# make sure to clean up on exit -trap "rm -f \"${fusionout}\" \"${fusionpipe}\" \"${waitout}\" \"${stats}\"" EXIT - -# launch p4-fusion in the background, sending all output to the pipe for capture and re-echoing -# depends on the p4-fusion binary executable being copied to p4-fusion-binary in the gitserver Dockerfile -p4-fusion-binary "${@}" >"${fusionpipe}" 2>&1 & - -# capture the pid of the child process -fpid=$! - -# start up a "sidecar" process to capture resource usage. -# it will terminate when the p4-fusion process terminates. -process-stats-watcher.sh "${fpid}" "p4-fusion-binary" >"${stats}" & -spid=$! - -# Wait for the child process to finish -wait ${fpid} >"${waitout}" 2>&1 - -# capture the result of the wait, which is the result of the child process -# or the result of external action on the child process, like SIGKILL -waitcode=$? - -# the sidecar process should have exited by now, but just in case, wait for it -wait "${spid}" >/dev/null 2>&1 - -[ ${waitcode} -eq 0 ] || { - # if the wait exit code indicates a problem, - # check to see if the child process was killed - killed="" - # if the process was killed with SIGKILL, the `wait` process will have generated a notification - grep -qs "Killed" "${waitout}" && killed=y - [ -z "${killed}" ] && { - # If the wait process did not generate an error message, check the process output. - # The process traps SIGINT and SIGTERM; uncaught signals will be displayed as "uncaught" - tail -5 "${fusionout}" | grep -Eqs "Signal Received:|uncaught target signal" && killed=y - } - [ -z "${killed}" ] || { - # include the signal if it's SIGINT, SIGTERM, or SIGKILL - # not gauranteed to work, but nice if we can include the info - signal="$(kill -l ${waitcode})" - [ -z "${signal}" ] || signal=" (SIG${signal})" - # get info if available from the sidecar process - rusage="" - [ -s "${stats}" ] && { - # expect the last (maybe only) line to be four fields: - # RSS VSZ ETIME TIME - x=($(tail -1 "${stats}")) - # NOTE: bash indexes from 0; zsh indexes from 1 - [ ${#x[@]} -eq 4 ] && rusage=" At the time of its demise, it had been running for ${x[2]}, had used ${x[3]} CPU time, reserved ${x[1]} RAM and was using ${x[0]}." - } - echo "p4-fusion was killed by an external signal${signal}.${rusage}" - } -} - -exit ${waitcode} diff --git a/cmd/gitserver/process-stats-watcher.sh b/cmd/gitserver/process-stats-watcher.sh deleted file mode 100755 index 7b24e5d196bd..000000000000 --- a/cmd/gitserver/process-stats-watcher.sh +++ /dev/null @@ -1,45 +0,0 @@ -#!/usr/bin/env bash -# shellcheck disable=SC2064,SC2207,SC2009 - -humanize() { - local num=${1} - [[ ${num} =~ ^[0-9][0-9]*$ ]] && num=$(bc <<<"scale=2;${num}/1024/1024")m - printf -- '%s' "${num}" - return 0 -} - -# read resource usage statistics for a process -# several times a second until it terminates -# at which point, output the most recent stats on stdout -# the output format is "RSS VSZ ETIME TIME" - -# input is the pid of the process -pid="${1}" -# and its name, which is used to avoid tracking -# another process in case the original process completed, -# and another started up and got assigned the same pid -cmd="${2}" - -unset rss vsz etime time - -while true; do - # Alpine has a rather limited `ps` - # it does not limit output to just one process, even when specifying a pid - # so we need to filter the output by pid - x="$(ps -o pid,stat,rss,vsz,etime,time,comm,args "${pid}" | grep "^ *${pid} " | grep "${cmd}" | tail -1)" - [ -z "${x}" ] && break - IFS=" " read -r -a a <<<"$x" - # drop out of here if the process has died or become a zombie - no coming back from the dead - [[ "${a[1]}" =~ ^[ZXx] ]] && break - # only collect stats for processes that are active (running, sleeping, disk sleep, which is waiting for I/O to complete) - # but don't stop until it is really is dead - [[ "${a[1]}" =~ ^[RSD] ]] && { - rss=${a[2]} - vsz=${a[3]} - etime=${a[4]} - time=${a[5]} - } - sleep 0.2 -done - -printf '%s %s %s %s' "$(humanize "${rss}")" "$(humanize "${vsz}")" "${etime}" "${time}" diff --git a/cmd/server/BUILD.bazel b/cmd/server/BUILD.bazel index 8e417b46e16e..14afd9e2b97d 100644 --- a/cmd/server/BUILD.bazel +++ b/cmd/server/BUILD.bazel @@ -177,7 +177,6 @@ oci_image( ":tar_postgres_optimize", ":tar_postgres_reindex", ":tar_prom-wrapper", - "//cmd/gitserver:tar_p4_fusion_wrappers", "//monitoring:generate_grafana_config_tar", "tar_image_test_scripts", ] + dependencies_tars(DEPS) + dependencies_tars(ZOEKT_DEPS), diff --git a/cmd/server/image_test.yaml b/cmd/server/image_test.yaml index e0bd9717908b..8091e31cfd11 100644 --- a/cmd/server/image_test.yaml +++ b/cmd/server/image_test.yaml @@ -32,7 +32,7 @@ commandTests: # P4-fusion - name: "p4-fusion is runnable" - command: "p4-fusion-binary" + command: "p4-fusion" args: expectedOutput: [ "--noBaseCommit" ] exitCode: 1 @@ -89,19 +89,6 @@ fileExistenceTests: # uid: 0 # gid: 0 # permissions: '-r-xr-xr-x' -# p4-fusion wrappers -- name: '/usr/local/bin/p4-fusion' - path: '/usr/local/bin/p4-fusion' - shouldExist: true - uid: 0 - gid: 0 - permissions: '-r-xr-xr-x' -- name: '/usr/local/bin/process-stats-watcher.sh' - path: '/usr/local/bin/process-stats-watcher.sh' - shouldExist: true - uid: 0 - gid: 0 - permissions: '-r-xr-xr-x' # /sg_config_prometheus configuration - name: '/sg_config_prometheus/prometheus.yml' path: '/sg_config_prometheus/prometheus.yml' diff --git a/cmd/symbols/gitserver/client.go b/cmd/symbols/gitserver/client.go index 4b39e4b11239..f59ca56c072d 100644 --- a/cmd/symbols/gitserver/client.go +++ b/cmd/symbols/gitserver/client.go @@ -101,8 +101,30 @@ func (c *gitserverClient) LogReverseEach(ctx context.Context, repo string, commi return c.innerClient.LogReverseEach(ctx, repo, commit, n, onLogEntry) } +const revListPageSize = 100 + func (c *gitserverClient) RevList(ctx context.Context, repo string, commit string, onCommit func(commit string) (shouldContinue bool, err error)) error { - return c.innerClient.RevList(ctx, repo, commit, onCommit) + nextCursor := commit + for { + var commits []api.CommitID + var err error + commits, nextCursor, err = c.innerClient.RevList(ctx, api.RepoName(repo), nextCursor, revListPageSize) + if err != nil { + return err + } + for _, c := range commits { + shouldContinue, err := onCommit(string(c)) + if err != nil { + return err + } + if !shouldContinue { + return nil + } + } + if nextCursor == "" { + return nil + } + } } var NUL = []byte{0} diff --git a/internal/batches/sources/mocks_test.go b/internal/batches/sources/mocks_test.go index 5fbdab1e8ce1..bcb46d7d145e 100644 --- a/internal/batches/sources/mocks_test.go +++ b/internal/batches/sources/mocks_test.go @@ -11296,7 +11296,7 @@ func NewMockGitserverClient() *MockGitserverClient { }, }, RevListFunc: &GitserverClientRevListFunc{ - defaultHook: func(context.Context, string, string, func(commit string) (bool, error)) (r0 error) { + defaultHook: func(context.Context, api.RepoName, string, int) (r0 []api.CommitID, r1 string, r2 error) { return }, }, @@ -11518,7 +11518,7 @@ func NewStrictMockGitserverClient() *MockGitserverClient { }, }, RevListFunc: &GitserverClientRevListFunc{ - defaultHook: func(context.Context, string, string, func(commit string) (bool, error)) error { + defaultHook: func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) { panic("unexpected invocation of MockGitserverClient.RevList") }, }, @@ -15769,24 +15769,24 @@ func (c GitserverClientRevAtTimeFuncCall) Results() []interface{} { // GitserverClientRevListFunc describes the behavior when the RevList method // of the parent MockGitserverClient instance is invoked. type GitserverClientRevListFunc struct { - defaultHook func(context.Context, string, string, func(commit string) (bool, error)) error - hooks []func(context.Context, string, string, func(commit string) (bool, error)) error + defaultHook func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) + hooks []func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) history []GitserverClientRevListFuncCall mutex sync.Mutex } // RevList delegates to the next hook function in the queue and stores the // parameter and result values of this invocation. -func (m *MockGitserverClient) RevList(v0 context.Context, v1 string, v2 string, v3 func(commit string) (bool, error)) error { - r0 := m.RevListFunc.nextHook()(v0, v1, v2, v3) - m.RevListFunc.appendCall(GitserverClientRevListFuncCall{v0, v1, v2, v3, r0}) - return r0 +func (m *MockGitserverClient) RevList(v0 context.Context, v1 api.RepoName, v2 string, v3 int) ([]api.CommitID, string, error) { + r0, r1, r2 := m.RevListFunc.nextHook()(v0, v1, v2, v3) + m.RevListFunc.appendCall(GitserverClientRevListFuncCall{v0, v1, v2, v3, r0, r1, r2}) + return r0, r1, r2 } // SetDefaultHook sets function that is called when the RevList method of // the parent MockGitserverClient instance is invoked and the hook queue is // empty. -func (f *GitserverClientRevListFunc) SetDefaultHook(hook func(context.Context, string, string, func(commit string) (bool, error)) error) { +func (f *GitserverClientRevListFunc) SetDefaultHook(hook func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error)) { f.defaultHook = hook } @@ -15794,7 +15794,7 @@ func (f *GitserverClientRevListFunc) SetDefaultHook(hook func(context.Context, s // RevList method of the parent MockGitserverClient instance invokes the // hook at the front of the queue and discards it. After the queue is empty, // the default hook function is invoked for any future action. -func (f *GitserverClientRevListFunc) PushHook(hook func(context.Context, string, string, func(commit string) (bool, error)) error) { +func (f *GitserverClientRevListFunc) PushHook(hook func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error)) { f.mutex.Lock() f.hooks = append(f.hooks, hook) f.mutex.Unlock() @@ -15802,20 +15802,20 @@ func (f *GitserverClientRevListFunc) PushHook(hook func(context.Context, string, // SetDefaultReturn calls SetDefaultHook with a function that returns the // given values. -func (f *GitserverClientRevListFunc) SetDefaultReturn(r0 error) { - f.SetDefaultHook(func(context.Context, string, string, func(commit string) (bool, error)) error { - return r0 +func (f *GitserverClientRevListFunc) SetDefaultReturn(r0 []api.CommitID, r1 string, r2 error) { + f.SetDefaultHook(func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) { + return r0, r1, r2 }) } // PushReturn calls PushHook with a function that returns the given values. -func (f *GitserverClientRevListFunc) PushReturn(r0 error) { - f.PushHook(func(context.Context, string, string, func(commit string) (bool, error)) error { - return r0 +func (f *GitserverClientRevListFunc) PushReturn(r0 []api.CommitID, r1 string, r2 error) { + f.PushHook(func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) { + return r0, r1, r2 }) } -func (f *GitserverClientRevListFunc) nextHook() func(context.Context, string, string, func(commit string) (bool, error)) error { +func (f *GitserverClientRevListFunc) nextHook() func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) { f.mutex.Lock() defer f.mutex.Unlock() @@ -15853,16 +15853,22 @@ type GitserverClientRevListFuncCall struct { Arg0 context.Context // Arg1 is the value of the 2nd argument passed to this method // invocation. - Arg1 string + Arg1 api.RepoName // Arg2 is the value of the 3rd argument passed to this method // invocation. Arg2 string // Arg3 is the value of the 4th argument passed to this method // invocation. - Arg3 func(commit string) (bool, error) + Arg3 int // Result0 is the value of the 1st result returned from this method // invocation. - Result0 error + Result0 []api.CommitID + // Result1 is the value of the 2nd result returned from this method + // invocation. + Result1 string + // Result2 is the value of the 3rd result returned from this method + // invocation. + Result2 error } // Args returns an interface slice containing the arguments of this @@ -15874,7 +15880,7 @@ func (c GitserverClientRevListFuncCall) Args() []interface{} { // Results returns an interface slice containing the results of this // invocation. func (c GitserverClientRevListFuncCall) Results() []interface{} { - return []interface{}{c.Result0} + return []interface{}{c.Result0, c.Result1, c.Result2} } // GitserverClientScopedFunc describes the behavior when the Scoped method diff --git a/internal/gitserver/BUILD.bazel b/internal/gitserver/BUILD.bazel index 6d729b549c97..ca399573fc26 100644 --- a/internal/gitserver/BUILD.bazel +++ b/internal/gitserver/BUILD.bazel @@ -23,6 +23,7 @@ go_library( "//internal/actor", "//internal/api", "//internal/authz", + "//internal/byteutils", "//internal/conf", "//internal/conf/conftypes", "//internal/extsvc/gitolite", diff --git a/internal/gitserver/client.go b/internal/gitserver/client.go index 1441bca23922..24c4a5bb625b 100644 --- a/internal/gitserver/client.go +++ b/internal/gitserver/client.go @@ -494,9 +494,10 @@ type Client interface { // LogReverseEach runs git log in reverse order and calls the given callback for each entry. LogReverseEach(ctx context.Context, repo string, commit string, n int, onLogEntry func(entry gitdomain.LogEntry) error) error - // RevList makes a git rev-list call and iterates through the resulting commits, calling the provided - // onCommit function for each. - RevList(ctx context.Context, repo string, commit string, onCommit func(commit string) (bool, error)) error + // RevList makes a git rev-list call and returns up to count commits. if nextCursor + // is non-empty, it is used as the starting point for the next call, use it to iterate + // to iterate over the whole history. + RevList(ctx context.Context, repo api.RepoName, commit string, count int) (_ []api.CommitID, nextCursor string, err error) // SystemsInfo returns information about all gitserver instances associated with a Sourcegraph instance. SystemsInfo(ctx context.Context) ([]protocol.SystemInfo, error) diff --git a/internal/gitserver/commands.go b/internal/gitserver/commands.go index 1a6a8cc705e5..6bb75816f5c1 100644 --- a/internal/gitserver/commands.go +++ b/internal/gitserver/commands.go @@ -29,6 +29,7 @@ import ( "github.com/sourcegraph/sourcegraph/internal/actor" "github.com/sourcegraph/sourcegraph/internal/api" "github.com/sourcegraph/sourcegraph/internal/authz" + "github.com/sourcegraph/sourcegraph/internal/byteutils" "github.com/sourcegraph/sourcegraph/internal/fileutil" "github.com/sourcegraph/sourcegraph/internal/gitserver/gitdomain" proto "github.com/sourcegraph/sourcegraph/internal/gitserver/v1" @@ -1107,30 +1108,51 @@ func (c *clientImplementor) MergeBase(ctx context.Context, repo api.RepoName, ba return api.CommitID(res.GetMergeBaseCommitSha()), nil } -// RevList makes a git rev-list call and iterates through the resulting commits, calling the provided onCommit function for each. -func (c *clientImplementor) RevList(ctx context.Context, repo string, commit string, onCommit func(commit string) (shouldContinue bool, err error)) (err error) { +func (c *clientImplementor) RevList(ctx context.Context, repo api.RepoName, commit string, count int) (_ []api.CommitID, nextCursor string, err error) { ctx, _, endObservation := c.operations.revList.With(ctx, &err, observation.Args{ MetricLabelValues: []string{c.scope}, Attrs: []attribute.KeyValue{ - attribute.String("repo", repo), + repo.Attr(), attribute.String("commit", commit), }, }) defer endObservation(1, observation.Args{}) - command := c.gitCommand(api.RepoName(repo), RevListArgs(commit)...) - command.DisableTimeout() - stdout, err := command.StdoutReader(ctx) + command := c.gitCommand(repo, revListArgs(count, commit)...) + + stdout, err := command.Output(ctx) if err != nil { - return err + return nil, "", err } - defer stdout.Close() - return gitdomain.RevListEach(stdout, onCommit) + commits := make([]api.CommitID, 0, count+1) + + lr := byteutils.NewLineReader(stdout) + for lr.Scan() { + line := lr.Line() + line = bytes.TrimSpace(line) + if len(line) == 0 { + continue + } + commit := api.CommitID(line) + commits = append(commits, commit) + } + + if len(commits) > count { + nextCursor = string(commits[len(commits)-1]) + commits = commits[:count] + } + + return commits, nextCursor, nil } -func RevListArgs(givenCommit string) []string { - return []string{"rev-list", "--first-parent", givenCommit} +func revListArgs(count int, givenCommit string) []string { + return []string{ + "rev-list", + "--first-parent", + fmt.Sprintf("--max-count=%d", count+1), + givenCommit, + } } // GetBehindAhead returns the behind/ahead commit counts information for right vs. left (both Git diff --git a/internal/gitserver/commands_test.go b/internal/gitserver/commands_test.go index b382db81ebb1..3775a948dace 100644 --- a/internal/gitserver/commands_test.go +++ b/internal/gitserver/commands_test.go @@ -2254,3 +2254,57 @@ func TestClient_FirstEverCommit(t *testing.T) { }) }) } + +func TestRevList(t *testing.T) { + ClientMocks.LocalGitserver = true + defer ResetClientMocks() + + gitCommands := []string{ + "git commit --allow-empty -m commit1", + "git commit --allow-empty -m commit2", + "git commit --allow-empty -m commit3", + } + repo := MakeGitRepository(t, gitCommands...) + allCommits := []api.CommitID{ + "4ac04f2761285633cd35188c696a6e08de03c00c", + "e7d0b23cb4e2e975ad657b163793bc83926c21b2", + "a04652fa1998a0a7d2f2f77ecb7021de943d3aab", + } + + t.Run("returns commits in reverse chronological order", func(t *testing.T) { + client := NewTestClient(t) + commits, _, err := client.RevList(context.Background(), repo, "HEAD", 999) + require.NoError(t, err) + require.Equal(t, allCommits, commits) + }) + + t.Run("returns next cursor when more commits exist", func(t *testing.T) { + gitCommands := []string{ + "git commit --allow-empty -m commit1", + "git commit --allow-empty -m commit2", + "git commit --allow-empty -m commit3", + } + repo := MakeGitRepository(t, gitCommands...) + client := NewTestClient(t) + nextCursor := "HEAD" + haveCommits := []api.CommitID{} + for { + commits, next, err := client.RevList(context.Background(), repo, nextCursor, 1) + require.NoError(t, err) + nextCursor = next + haveCommits = append(haveCommits, commits...) + if nextCursor == "" { + break + } + } + require.Equal(t, allCommits, haveCommits) + }) + + t.Run("returns error when commit does not exist", func(t *testing.T) { + repo := MakeGitRepository(t) + client := NewTestClient(t) + _, _, err := client.RevList(context.Background(), repo, "nonexistent", 10) + require.Error(t, err) + require.Contains(t, err.Error(), "exit status 128") + }) +} diff --git a/internal/gitserver/gitdomain/log.go b/internal/gitserver/gitdomain/log.go index 2ac971f9d988..86be498415a8 100644 --- a/internal/gitserver/gitdomain/log.go +++ b/internal/gitserver/gitdomain/log.go @@ -44,26 +44,6 @@ func LogReverseArgs(n int, givenCommit string) []string { } } -func RevListEach(stdout io.Reader, onCommit func(commit string) (shouldContinue bool, err error)) error { - reader := bufio.NewReader(stdout) - - for { - commit, err := reader.ReadString('\n') - if err == io.EOF { - break - } else if err != nil { - return err - } - commit = commit[:len(commit)-1] // Drop the trailing newline - shouldContinue, err := onCommit(commit) - if !shouldContinue { - return err - } - } - - return nil -} - func ParseLogReverseEach(stdout io.Reader, onLogEntry func(entry LogEntry) error) error { reader := bufio.NewReader(stdout) diff --git a/internal/gitserver/mocks_temp.go b/internal/gitserver/mocks_temp.go index b2ba2c5869e1..180c0c58343e 100644 --- a/internal/gitserver/mocks_temp.go +++ b/internal/gitserver/mocks_temp.go @@ -340,7 +340,7 @@ func NewMockClient() *MockClient { }, }, RevListFunc: &ClientRevListFunc{ - defaultHook: func(context.Context, string, string, func(commit string) (bool, error)) (r0 error) { + defaultHook: func(context.Context, api.RepoName, string, int) (r0 []api.CommitID, r1 string, r2 error) { return }, }, @@ -562,7 +562,7 @@ func NewStrictMockClient() *MockClient { }, }, RevListFunc: &ClientRevListFunc{ - defaultHook: func(context.Context, string, string, func(commit string) (bool, error)) error { + defaultHook: func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) { panic("unexpected invocation of MockClient.RevList") }, }, @@ -4743,23 +4743,23 @@ func (c ClientRevAtTimeFuncCall) Results() []interface{} { // ClientRevListFunc describes the behavior when the RevList method of the // parent MockClient instance is invoked. type ClientRevListFunc struct { - defaultHook func(context.Context, string, string, func(commit string) (bool, error)) error - hooks []func(context.Context, string, string, func(commit string) (bool, error)) error + defaultHook func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) + hooks []func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) history []ClientRevListFuncCall mutex sync.Mutex } // RevList delegates to the next hook function in the queue and stores the // parameter and result values of this invocation. -func (m *MockClient) RevList(v0 context.Context, v1 string, v2 string, v3 func(commit string) (bool, error)) error { - r0 := m.RevListFunc.nextHook()(v0, v1, v2, v3) - m.RevListFunc.appendCall(ClientRevListFuncCall{v0, v1, v2, v3, r0}) - return r0 +func (m *MockClient) RevList(v0 context.Context, v1 api.RepoName, v2 string, v3 int) ([]api.CommitID, string, error) { + r0, r1, r2 := m.RevListFunc.nextHook()(v0, v1, v2, v3) + m.RevListFunc.appendCall(ClientRevListFuncCall{v0, v1, v2, v3, r0, r1, r2}) + return r0, r1, r2 } // SetDefaultHook sets function that is called when the RevList method of // the parent MockClient instance is invoked and the hook queue is empty. -func (f *ClientRevListFunc) SetDefaultHook(hook func(context.Context, string, string, func(commit string) (bool, error)) error) { +func (f *ClientRevListFunc) SetDefaultHook(hook func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error)) { f.defaultHook = hook } @@ -4767,7 +4767,7 @@ func (f *ClientRevListFunc) SetDefaultHook(hook func(context.Context, string, st // RevList method of the parent MockClient instance invokes the hook at the // front of the queue and discards it. After the queue is empty, the default // hook function is invoked for any future action. -func (f *ClientRevListFunc) PushHook(hook func(context.Context, string, string, func(commit string) (bool, error)) error) { +func (f *ClientRevListFunc) PushHook(hook func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error)) { f.mutex.Lock() f.hooks = append(f.hooks, hook) f.mutex.Unlock() @@ -4775,20 +4775,20 @@ func (f *ClientRevListFunc) PushHook(hook func(context.Context, string, string, // SetDefaultReturn calls SetDefaultHook with a function that returns the // given values. -func (f *ClientRevListFunc) SetDefaultReturn(r0 error) { - f.SetDefaultHook(func(context.Context, string, string, func(commit string) (bool, error)) error { - return r0 +func (f *ClientRevListFunc) SetDefaultReturn(r0 []api.CommitID, r1 string, r2 error) { + f.SetDefaultHook(func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) { + return r0, r1, r2 }) } // PushReturn calls PushHook with a function that returns the given values. -func (f *ClientRevListFunc) PushReturn(r0 error) { - f.PushHook(func(context.Context, string, string, func(commit string) (bool, error)) error { - return r0 +func (f *ClientRevListFunc) PushReturn(r0 []api.CommitID, r1 string, r2 error) { + f.PushHook(func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) { + return r0, r1, r2 }) } -func (f *ClientRevListFunc) nextHook() func(context.Context, string, string, func(commit string) (bool, error)) error { +func (f *ClientRevListFunc) nextHook() func(context.Context, api.RepoName, string, int) ([]api.CommitID, string, error) { f.mutex.Lock() defer f.mutex.Unlock() @@ -4826,16 +4826,22 @@ type ClientRevListFuncCall struct { Arg0 context.Context // Arg1 is the value of the 2nd argument passed to this method // invocation. - Arg1 string + Arg1 api.RepoName // Arg2 is the value of the 3rd argument passed to this method // invocation. Arg2 string // Arg3 is the value of the 4th argument passed to this method // invocation. - Arg3 func(commit string) (bool, error) + Arg3 int // Result0 is the value of the 1st result returned from this method // invocation. - Result0 error + Result0 []api.CommitID + // Result1 is the value of the 2nd result returned from this method + // invocation. + Result1 string + // Result2 is the value of the 3rd result returned from this method + // invocation. + Result2 error } // Args returns an interface slice containing the arguments of this @@ -4847,7 +4853,7 @@ func (c ClientRevListFuncCall) Args() []interface{} { // Results returns an interface slice containing the results of this // invocation. func (c ClientRevListFuncCall) Results() []interface{} { - return []interface{}{c.Result0} + return []interface{}{c.Result0, c.Result1, c.Result2} } // ClientScopedFunc describes the behavior when the Scoped method of the diff --git a/internal/rockskip/BUILD.bazel b/internal/rockskip/BUILD.bazel index 724bf1ef185f..9f4c741ead65 100644 --- a/internal/rockskip/BUILD.bazel +++ b/internal/rockskip/BUILD.bazel @@ -64,5 +64,6 @@ go_test( "//lib/pointers", "@com_github_google_go_cmp//cmp", "@com_github_sourcegraph_go_ctags//:go-ctags", + "@com_github_stretchr_testify//require", ], ) diff --git a/internal/rockskip/server_test.go b/internal/rockskip/server_test.go index cf9396af7b26..d5034efb68fb 100644 --- a/internal/rockskip/server_test.go +++ b/internal/rockskip/server_test.go @@ -2,6 +2,7 @@ package rockskip import ( "bufio" + "bytes" "context" "fmt" "io" @@ -15,6 +16,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/sourcegraph/go-ctags" + "github.com/stretchr/testify/require" "github.com/sourcegraph/sourcegraph/cmd/symbols/fetcher" "github.com/sourcegraph/sourcegraph/internal/api" @@ -45,50 +47,22 @@ func (mockParser) Parse(path string, bytes []byte) ([]*ctags.Entry, error) { func (mockParser) Close() {} func TestIndex(t *testing.T) { - fatalIfError := func(err error, message string) { - if err != nil { - t.Fatal(errors.Wrap(err, message)) - } - } - - gitDir, err := os.MkdirTemp("", "rockskip-test-index") - fatalIfError(err, "faiMkdirTemp") + gitserver.ClientMocks.LocalGitserver = true + t.Cleanup(gitserver.ResetClientMocks) + repo, repoDir := gitserver.MakeGitRepositoryAndReturnDir(t) - t.Cleanup(func() { - if t.Failed() { - t.Logf("git dir %s left intact for inspection", gitDir) - } else { - os.RemoveAll(gitDir) - } - }) - - gitCmd := func(args ...string) *exec.Cmd { - cmd := exec.Command("git", args...) - cmd.Dir = gitDir - return cmd - } + state := map[string][]string{} gitRun := func(args ...string) { - fatalIfError(gitCmd(args...).Run(), "git "+strings.Join(args, " ")) + out, err := gitserver.CreateGitCommand(repoDir, "git", args...).CombinedOutput() + require.NoError(t, err, string(out)) } - gitStdout := func(args ...string) string { - stdout, err := gitCmd(args...).Output() - fatalIfError(err, "git "+strings.Join(args, " ")) - return string(stdout) - } - - getHead := func() string { - return strings.TrimSpace(gitStdout("rev-parse", "HEAD")) - } - - state := map[string][]string{} - add := func(filename string, contents string) { - fatalIfError(os.WriteFile(path.Join(gitDir, filename), []byte(contents), 0644), "os.WriteFile") + require.NoError(t, os.WriteFile(path.Join(repoDir, filename), []byte(contents), 0644), "os.WriteFile") gitRun("add", filename) symbols, err := mockParser{}.Parse(filename, []byte(contents)) - fatalIfError(err, "simpleParse") + require.NoError(t, err) state[filename] = []string{} for _, symbol := range symbols { state[filename] = append(state[filename], symbol.Name) @@ -104,8 +78,8 @@ func TestIndex(t *testing.T) { // Needed in CI gitRun("config", "user.email", "test@sourcegraph.com") - git, err := NewSubprocessGit(gitDir) - fatalIfError(err, "NewSubprocessGit") + git, err := NewSubprocessGit(t, repoDir) + require.NoError(t, err) defer git.Close() db := dbtest.NewDB(t) @@ -114,18 +88,20 @@ func TestIndex(t *testing.T) { createParser := func() (ctags.Parser, error) { return mockParser{}, nil } service, err := NewService(db, git, newMockRepositoryFetcher(git), createParser, 1, 1, false, 1, 1, 1, false) - fatalIfError(err, "NewService") + require.NoError(t, err) verifyBlobs := func() { - repo := "somerepo" - commit := getHead() + out, err := gitserver.CreateGitCommand(repoDir, "git", "rev-parse", "HEAD").CombinedOutput() + require.NoError(t, err, string(out)) + commit := string(bytes.TrimSpace(out)) + args := search.SymbolsParameters{ Repo: api.RepoName(repo), CommitID: api.CommitID(commit), Query: "", IncludeLangs: []string{"Text"}} symbols, err := service.Search(context.Background(), args) - fatalIfError(err, "Search") + require.NoError(t, err) // Make sure the paths match. gotPathSet := map[string]struct{}{} @@ -149,7 +125,7 @@ func TestIndex(t *testing.T) { fmt.Println("unexpected paths (-got +want)") fmt.Println(diff) err = PrintInternals(context.Background(), db) - fatalIfError(err, "PrintInternals") + require.NoError(t, err) t.FailNow() } @@ -167,7 +143,7 @@ func TestIndex(t *testing.T) { fmt.Println("unexpected symbols (-got +want)") fmt.Println(diff) err = PrintInternals(context.Background(), db) - fatalIfError(err, "PrintInternals") + require.NoError(t, err) t.FailNow() } } @@ -204,9 +180,10 @@ type SubprocessGit struct { catFileCmd *exec.Cmd catFileStdin io.WriteCloser catFileStdout bufio.Reader + gs gitserver.Client } -func NewSubprocessGit(gitDir string) (*SubprocessGit, error) { +func NewSubprocessGit(t testing.TB, gitDir string) (*SubprocessGit, error) { cmd := exec.Command("git", "cat-file", "--batch") cmd.Dir = gitDir @@ -230,6 +207,7 @@ func NewSubprocessGit(gitDir string) (*SubprocessGit, error) { catFileCmd: cmd, catFileStdin: stdin, catFileStdout: *bufio.NewReader(stdout), + gs: gitserver.NewTestClient(t), }, nil } @@ -263,26 +241,28 @@ func (g SubprocessGit) LogReverseEach(ctx context.Context, repo string, givenCom return gitdomain.ParseLogReverseEach(output, onLogEntry) } -func (g SubprocessGit) RevList(ctx context.Context, repo string, givenCommit string, onCommit func(commit string) (shouldContinue bool, err error)) (returnError error) { - revList := exec.Command("git", gitserver.RevListArgs(givenCommit)...) - revList.Dir = g.gitDir - output, err := revList.StdoutPipe() - if err != nil { - return err - } - - err = revList.Start() - if err != nil { - return err - } - defer func() { - err = revList.Wait() +func (g *SubprocessGit) RevList(ctx context.Context, repo string, commit string, onCommit func(commit string) (shouldContinue bool, err error)) (returnError error) { + nextCursor := commit + for { + var commits []api.CommitID + var err error + commits, nextCursor, err = g.gs.RevList(ctx, api.RepoName(repo), nextCursor, 100) if err != nil { - returnError = err + return err } - }() - - return gitdomain.RevListEach(output, onCommit) + for _, c := range commits { + shouldContinue, err := onCommit(string(c)) + if err != nil { + return err + } + if !shouldContinue { + return nil + } + } + if nextCursor == "" { + return nil + } + } } func newMockRepositoryFetcher(git *SubprocessGit) fetcher.RepositoryFetcher { diff --git a/wolfi-images/gitserver.lock.json b/wolfi-images/gitserver.lock.json index 77541cbe50de..22e29c2f67ab 100755 --- a/wolfi-images/gitserver.lock.json +++ b/wolfi-images/gitserver.lock.json @@ -964,22 +964,22 @@ }, { "architecture": "x86_64", - "checksum": "Q1LbqvA8R46fS+Zwl18guXt5zvYrw=", + "checksum": "Q1NyPDKnpnwrKJ7+7foWCAaDw7V5s=", "control": { - "checksum": "sha1-LbqvA8R46fS+Zwl18guXt5zvYrw=", - "range": "bytes=660-1037" + "checksum": "sha1-NyPDKnpnwrKJ7+7foWCAaDw7V5s=", + "range": "bytes=660-1036" }, "data": { - "checksum": "sha256-Onbp332eMogVPHfUpduSsjA5w+xpyHcCb7VuMA4T1x8=", - "range": "bytes=1038-20466045" + "checksum": "sha256-4VGdMs4AxZGZuJXWW2ztq3UWSnWb0z6+Aa4s5uInmzI=", + "range": "bytes=1037-20466010" }, "name": "p4-fusion-sg", "signature": { - "checksum": "sha1-29MX0Y8RvdpmHPm4IPFCo3diYzY=", + "checksum": "sha1-3jfZRXx840UdCOYzfJKSoffWnJY=", "range": "bytes=0-659" }, - "url": "https://packages.sgdev.org/main/x86_64/p4-fusion-sg-1.13.2-r3.apk", - "version": "1.13.2-r3" + "url": "https://packages.sgdev.org/main/x86_64/p4-fusion-sg-1.13.2-r4.apk", + "version": "1.13.2-r4" }, { "architecture": "x86_64", diff --git a/wolfi-images/server.lock.json b/wolfi-images/server.lock.json index 8c322e7a13f2..9f8ee314a0d3 100755 --- a/wolfi-images/server.lock.json +++ b/wolfi-images/server.lock.json @@ -1424,22 +1424,22 @@ }, { "architecture": "x86_64", - "checksum": "Q1LbqvA8R46fS+Zwl18guXt5zvYrw=", + "checksum": "Q1NyPDKnpnwrKJ7+7foWCAaDw7V5s=", "control": { - "checksum": "sha1-LbqvA8R46fS+Zwl18guXt5zvYrw=", - "range": "bytes=660-1037" + "checksum": "sha1-NyPDKnpnwrKJ7+7foWCAaDw7V5s=", + "range": "bytes=660-1036" }, "data": { - "checksum": "sha256-Onbp332eMogVPHfUpduSsjA5w+xpyHcCb7VuMA4T1x8=", - "range": "bytes=1038-20466045" + "checksum": "sha256-4VGdMs4AxZGZuJXWW2ztq3UWSnWb0z6+Aa4s5uInmzI=", + "range": "bytes=1037-20466010" }, "name": "p4-fusion-sg", "signature": { - "checksum": "sha1-29MX0Y8RvdpmHPm4IPFCo3diYzY=", + "checksum": "sha1-3jfZRXx840UdCOYzfJKSoffWnJY=", "range": "bytes=0-659" }, - "url": "https://packages.sgdev.org/main/x86_64/p4-fusion-sg-1.13.2-r3.apk", - "version": "1.13.2-r3" + "url": "https://packages.sgdev.org/main/x86_64/p4-fusion-sg-1.13.2-r4.apk", + "version": "1.13.2-r4" }, { "architecture": "x86_64",