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

Revert "Revert "c-deps: fix assertion-enabled builds"" #41153

Merged
merged 1 commit into from
Sep 28, 2019

Conversation

ajkr
Copy link
Contributor

@ajkr ajkr commented Sep 27, 2019

This reverts commit 6d6b5c4.

In this same PR I upgraded RocksDB to pull in
cockroachdb/rocksdb#59, which relaxes the
assertion whose failure led to the original revert.

Release justification: run make testrace in teamcity with rocksdb
assertions enabled to increase our bug-finding chances

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@petermattis petermattis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ajkr, @petermattis, and @tbg)


build/teamcity-testrace.sh, line 54 at r1 (raw file):

		TESTTIMEOUT=45m \
		TESTFLAGS='-v' \
		ENABLE_ROCKSDB_ASSERTIONS=1 2>&1 \

Should we pass in ENABLE_LIBROACH_ASSERTIONS=1 here too?

This reverts commit 6d6b5c4.

In the same PR I upgraded RocksDB to pull in
cockroachdb/rocksdb#59, which relaxes the
assertion whose failure led to the original revert.

Release justification: run `make testrace` in teamcity with rocksdb
assertions enabled to increase our bug-finding chances

Release note: None
Copy link
Contributor Author

@ajkr ajkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @petermattis and @tbg)


build/teamcity-testrace.sh, line 54 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Should we pass in ENABLE_LIBROACH_ASSERTIONS=1 here too?

Done.

@ajkr
Copy link
Contributor Author

ajkr commented Sep 28, 2019

bors r+

craig bot pushed a commit that referenced this pull request Sep 28, 2019
41153: Revert "Revert "c-deps: fix assertion-enabled builds"" r=ajkr a=ajkr

This reverts commit 6d6b5c4.

In this same PR I upgraded RocksDB to pull in
cockroachdb/rocksdb#59, which relaxes the
assertion whose failure led to the original revert.

Release justification: run `make testrace` in teamcity with rocksdb
assertions enabled to increase our bug-finding chances

Release note: None

Co-authored-by: Andrew Kryczka <andrew.kryczka2@gmail.com>
@craig
Copy link
Contributor

craig bot commented Sep 28, 2019

Build succeeded

@craig craig bot merged commit dd865fd into cockroachdb:master Sep 28, 2019
@knz knz mentioned this pull request Oct 2, 2019
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants