This fixes two small pain points with automated treatment of feature flags:
1) A user cannot add an override unless the feature flag has already been created and a default has been set. However, in all our code, we require the caller to specify a default if the feature flag doesn't exist, so this is not a necessary limitation. It's also particularly annoying because only site admins can create a feature flag.
2) In order to update an override, a user has to first fetch the override to see if it exists, then update it if it exists or create it if it doesn't. This modifies create to just overwrite if the override already exists.
* ci: Fetch tag before checkout in backcompat test
* Specify remote
* Specify --depth=1 to avoid over-fetching
* Test without --depth=1
* fetch tag specifically
---------
Co-authored-by: William Bezuidenhout <william.bezuidenhout@sourcegraph.com>
This removes contention around the single table. If you have a small
number of shards and lots of repositories that are cloned/recloned, they
all try to write to the same table concurrently.
What this does here is it changes the `gitserver_repos_statistics` table
to work like `repo_statistics`.
Co-authored-by: Erik Seliger <erikseliger@me.com>
* add patch to turn of race in backcompat
* git checkout with no-overlay
* fix patch
* move go_defs patch to own dir for 5.1.0
* add no-overlay to more git checkouts
* add 5.0.0 patches
* rm 5.0.0 patches since bazel was incomplete then
Main motivation is to see the effect on performance for attribution
search.
Note that this included a bump in the otel version used since zoekt is
using a new version. This had a bit of a cascading effect on our third
party deps since they removed a package. So this bumps most 3rd party
packages that directly interacted with otel.
The changed commits are
7643f3b313...45f608ff95
- a176bde1a3 go get -u -t ./...
- e2e8aede00 Fix template documentation comments
- 25c1ea5177 all: observe missing Stats RegexpsConsidered and FlushReason
- 9abbb8b0d3 zoekt-indexserver: Prevent invalid config from causing an NPE
- 008a775ba8 zoekt-indexserver: use value format directive for bad conf warning
- f9d3a0e2e4 zoekt: add fgprof for full profiling
- 3d0bdd5c9c remove ngram offset code
- 45f608ff95 sort ngrams before looking them up
Test Plan: tested in the zoekt repo. Our CI will handle the dep updates.
I eyeballed them and they all look low risk.
Co-authored-by: Jean-Hadrien Chabran <jh@chabran.fr>
- bumps `rules_go` to `0.40.1` (didn't go for `0.41.0` yet, the breaking
change with google apis is a bit messy to deal with, can be handled in a
second time).
- Because of an update to the crypto libs, we can't compare
authenticators directly anymore. Instead I updated the test code to
compare their `.Hash()`, which is semantically equivalent.
- Added that particular test to the flakes, to green the back compat
tests.
- Big thanks to @camdencheek for fixing the issues with the Go compiler
and the `execabs` mess which requires absolute paths, which needed to be
fixed, as Go 1.20.X doesn't ship with precompiled code for the stdlib
anymore.
## Test plan
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
CI
---------
Co-authored-by: Camden Cheek <camden@ccheek.com>
Minor improvements I noticed while updating this file. Not sure if this
is in good style for bazel files, so interested in feedback.
Test Plan: rely on CI, didn't test locally.
It seems we were using a weird version of throttled which I don't think
was actually based on the v2 series correctly. This updates us to the
latest version and moves us away from the removed struct GCRARateLimiter
and the deprecated interface GCRAStore.
I came across this while attempting to update deps and getting failing
compiles.
Test Plan: we have unit tests for this, so CI.
Movin enterprise codeintel stuff (and ~two others that had to be dragged
along) out of `enterprise/internal` and into `internal` as part of the
shift towards enterprise-only
## Test plan
Successfully built frontend with bazel, CI will check the rest
😎 no logic changed, just shufflin things around
This PR is on top of #54517
---
This PR rework how we handle the docsite and some of its related tasks:
- While we _could_ avoid committing the generated files to disk, the way
that docsite currently works, requires to have all files available in
git, so the docsite can serve various versions.
- `bazel run //doc/cli/references:write_doc_files` handles this (and
it's umbreall `//dev:write_all_generated`.
- Its generation is now handled by `//doc/cli/references:generate_doc`
and its associated test.
- `sg run docsite` now calls `bazel run //doc:serve`, we don't need to
install the docsite directly anymore (but it's available under `bazel
run //dev/tools:docsite` if needed).
As a byproduct, we can now run `src-cli` with `bazel run
//dev/tools:src-cli` which will pick the correct binary for your machine
transparently.
## Test plan
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
Locally tested + CI
Because we at least need one file to be present or the embed won't work,
this replaces CHANGELOG.md by CONTRIBUTING.md as a placeholder. This
will prevent PRs updating the changelog from invalidating the cache for
nothing for these slow targets.
## Test plan
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
CI
This endpoint was unused which was why it was removed in Zoekt. The
changes from Zoekt included in this dep bump:
- 1686b50d50 indexserver: remove unused GetRepoRank
- 7078a585a9 shards: populate RepoList.Stats.Repos
- b9e6d9433e zoekt-indexserver: Check stderr for git fetch
- 93f7b0c983 matchtree: capture Stats before pruning
- 7643f3b313 matchiter: capture metric NgramLookups
Test Plan: CI
This PR moves the target release for back compat tests to 5.1 and fixes
the various issues that arose.
- We don't need to handle otel breaking updates, as 5.1 and HEAD are on
the same version
- We now need to explicitly remove `/client/*` as we're properly
targeting the `v5.1.0` tag and it doesn't contain my crude patches
anymore
- Bumped pinned back compat scip deps, as it frequently breaks due to
having a lot of API changes across time
- Added a new patch, to handle the consequences of having removed the
client tree: previous release didn't have the refactored ui/assets
explicit oss/enterprise targets and thus didn't explicitly depend on the
client. Now we do, so it needed some patching to make the build working.
- The buildkite command itself now substracts a few steps that are not
playing very well with the backcompat approach, I'm not sure to
understand why, the the `//cmd/migrator:schema_descriptions` `genrule`
doesn't like very much being built in a different place. I suspect
that's because the tree is one folder below, but it's harder to patch
this than it is to simply remove it from the test targets entirely.
- And finally, the buildkite command also filters things to only run go
tests.
## Test plan
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
CI + local fun
This PR ships our freshly rewritten container images built with
rules_oci and Wolfi, which for now will only be used on S2.
*What is this about*
This work is the conjunction of [hardening container
images](https://github.com/orgs/sourcegraph/projects/302?pane=issue&itemId=25019223)
and fully building our container images with Bazel.
* All base images are now distroless, based on Wolfi, meaning we fully
control every little package version and we won't be subject anymore to
Alpine maintainers dropping a postgres version for example.
* Container images are now built with `rules_oci`, meaning we don't have
Dockerfile anymore, but instead created through [Bazel
rules](https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph@bzl/oci_wolfi/-/blob/enterprise/cmd/gitserver/BUILD.bazel).
Don't be scared, while this will look a bit strange to you at first,
it's much saner and simpler to do than our Dockerfiles and their muddy
shell scripts calling themselves in cascade.
:spiral_note_pad: *Plan*:
*1/ (NOW) We merge our branch on `main` today, here is what it does
change for you 👇:skin-tone-3::*
* On `main`:
* It will introduce a new job on `main` _Bazel Push_, which will push
those new images on our registries with all tags prefixed by `bazel-`.
* These new images will be picked up by S2 and S2 only.
* The existing jobs building docker images and pushing them will stay in
place until we have QA'ed them enough and are confident to roll them out
on Dotcom.
* Because we'll be building both images, there will be more jobs running
on `main`, but this should not affect the wall clock time.
* On all branches (so your PRs and `main`)
* The _Bazel Test_ job will now run: Backend Integration Tests, E2E
Tests and CodeIntel QA
* This will increase the duration of your test jobs in PRs, but as we
haven't removed yet the `sg lint` step, it should not affect too much
the wall clock time of your PRs.
* But it will also increase your confidence toward your changes, as the
coverage will vastly increased compared to before.
* If you have ongoing branches which are affecting the docker images
(like adding a new binary, like the recent `scip-tags`, reach us out on
#job-fair-bazel so we can help you to port your changes. It's much much
simpler than before, but it's going to be unfamiliar to you).
* If something goes awfully wrong, we'll rollback and update this
thread.
*2/ (EOW / Early next week) Once we're confident enough with what we saw
on S2, we'll roll the new images on Dotcom.*
* After the first successful deploy and a few sanity checks, we will
drop the old images building jobs.
* At this point, we'll reach out to all TLs asking for their help to
exercise all features of our product to ensure we catch any potential
breakage.
## Test plan
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
* We tested our new images on `scale-testing` and it worked.
* The new container building rules comes with _container tests_ which
ensures that produced images are containing and configured with what
should be in there:
[example](https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph@bzl/oci_wolfi/-/blob/enterprise/cmd/gitserver/image_test.yaml)
.
---------
Co-authored-by: Dave Try <davetry@gmail.com>
Co-authored-by: Will Dollman <will.dollman@sourcegraph.com>
CI fails on main without this.
Test Plan: ran locally and modified gen-pipeline to confirm it prints
the correct cwd.
Co-authored-by: Jean-Hadrien Chabran <jh@chabran.fr>
Even though caching is in place and should cover this, it's still can
flake on cold agents, which is annoying.
See
653cb673b7
This brings up the test target to use a backported patch to handle the
timeout issues.
## Test plan
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
Clones and patches Sourcegraph git repository under
@sourcegraph_back_compat so we can run tests targets from that
particular release against the new database schema.
Flakes can be defined to skip known problematic tests which are either
flaky or simply cannot run against the new schema in case of a breaking
change. See //dev/backcompat:flakes.bzl for more details about how to
define them.
The final result is the definition of a @sourcegraph_back_compat target,
whose test targets are exactly the same as back then, but with instead a
new schema.
Example: `bazel test
@sourcegraph_back_compat//enterprise/internal/batches/...`.
See
https://github.com/sourcegraph/sourcegraph/pull/50932/files#diff-2f07315ec320aa4080768fec54f32ebb2cbf4e3e6df7c51a314beda827c48c41R104
for the command generating the mandatory patch file in CI for these
tests to run.
If the patch file were to be missing, a placeholder diff is in place to
make it explicit (in the eventuality of someone running those locally).
A new CI job has been added because this one is fully independent from
the other targets to build, and will be cached most of the time.
Depending how it goes, I might bring that one over the main bazel job to
avoid getting an agent just for that.
Because it's cached, we can run this within PRs and have it take 10s max
for 99% of the PRs and main builds.
TODO: The flakefiles are still there, they're just defined in different
place. I need to port the annotation as well, that has proven to be
really useful.
Also right now it's a bit manual as the 5.0.0 target is a bit peculiar,
but that'll get much simpler as we progress, as the next old release
will be requiring less and less patching.
This turns about a dozen of 10m individual jobs into a single one
ranging from 30s if there's not database changes to 10m if there's some
of them and cold cache.
## Test plan
<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->
QA: fails on old code when a new migration breaks something:
-
https://buildkite.com/sourcegraph/sourcegraph/builds/214283#01879f2d-67be-4caf-b5d5-93f045e19348/118-126
