Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release,build: don't imply lintonbuild in ci configuration #138698

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rickystewart
Copy link
Collaborator

@rickystewart rickystewart commented Jan 8, 2025

The ci configuration is overloaded and is used for a few different things. It is used in builds to enable lintonbuild, and it's used in tests to set some configurations that are useful in TeamCity (increased verbosity, etc.)

While running lints every time we build in a paranoid, repetitive fashion does mean that lint issues cannot go unnoticed, it does have a performance impact (see #136626). Going forward, we will not have this configuration imply lintonbuild and provide the option to disable nogo for certain builds in automation. For example, roachtests will benefit from having lints disabled as well to speed up the build.

Here, we do the following:

  1. Have --config=ci not imply enabling lints
  2. Now, --config=ci has no specific meaning for builds (it's only meaningful for tests). So, I go through all the scripts and automation and remove it for builds specifically.
  3. Disable nogo for release builds to reduce the likelihood of an OOM.

Part of: #136626
Epic: CRDB-41952
Release note: None

@rickystewart rickystewart added the backport-24.3.x Flags PRs that need to be backported to 24.3 label Jan 8, 2025
@rickystewart rickystewart requested review from rail and jlinder January 8, 2025 23:37
@rickystewart rickystewart requested a review from a team as a code owner January 8, 2025 23:37
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rickystewart rickystewart force-pushed the ci-no-imply-validations branch from f3ce97b to 54e4a27 Compare January 8, 2025 23:41
The `ci` configuration is overloaded and is used for a few different
things. It is used in builds to enable `lintonbuild`, and it's used in
tests to set some configurations that are useful in TeamCity (increased
verbosity, etc.)

While running lints every time we build in a paranoid, repetitive
fashion does mean that lint issues cannot go unnoticed, it does have a
performance impact (see cockroachdb#136626). Going forward, we will not have this
configuration imply `lintonbuild` and provide the option to disable
`nogo` for certain builds in automation. For example, `roachtest`s will
benefit from having lints disabled as well to speed up the build.

Here, we do the following:

1. Have `--config=ci` not imply enabling lints
2. Now, `--config=ci` has no specific meaning for builds (it's only
   meaningful for tests). So, I go through all the scripts and
   automation and remove it for builds specifically.
3. Disable `nogo` for release builds to reduce the likelihood of an OOM.

Part of: cockroachdb#136626
Epic: CRDB-41952
Release note: None
@rickystewart
Copy link
Collaborator Author

bors r=rail

craig bot pushed a commit that referenced this pull request Jan 9, 2025
138698: release,build: don't imply `lintonbuild` in `ci` configuration r=rail a=rickystewart

The `ci` configuration is overloaded and is used for a few different things. It is used in builds to enable `lintonbuild`, and it's used in tests to set some configurations that are useful in TeamCity (increased verbosity, etc.)

While running lints every time we build in a paranoid, repetitive fashion does mean that lint issues cannot go unnoticed, it does have a performance impact (see #136626). Going forward, we will not have this configuration imply `lintonbuild` and provide the option to disable `nogo` for certain builds in automation. For example, `roachtest`s will benefit from having lints disabled as well to speed up the build.

Here, we do the following:

1. Have `--config=ci` not imply enabling lints
2. Now, `--config=ci` has no specific meaning for builds (it's only meaningful for tests). So, I go through all the scripts and automation and remove it for builds specifically.
3. Disable `nogo` for release builds to reduce the likelihood of an OOM.

Part of: #136626
Epic: CRDB-41952
Release note: None

138700: release: enable PGO on releases r=rail a=rickystewart

Part of: CRDB-44692
Epic: CRDB-41952
Release note: Build CRDB binary with PGO enabled

138702: roachtest: when building, don't run `nogo` r=rail a=rickystewart

Part of: #136626
Epic: None
Release note: None


138703: roachtest: build artifacts with PGO r=rail a=rickystewart

Epic: CRDB-41952
Release note: None

Co-authored-by: Ricky Stewart <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-24.3.x Flags PRs that need to be backported to 24.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants