Skip to content

makefile: fix rebuild IT binary condition #174

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

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

muzarski
Copy link
Collaborator

Currently, we rebuild (even if it exists) IT binary if DONT_REBUILD_INTEGRATION_BIN env var is non-empty. I believe it should be the opposite.

Pre-review checklist

  • I have split my patch into logically separate commits.
  • All commit messages clearly explain what they change and why.
  • PR description sums up the changes and reasons why they should be introduced.
  • [ ] I have enabled appropriate tests in .github/workflows/build.yml in gtest_filter.
  • [ ] I have enabled appropriate tests in .github/workflows/cassandra.yml in gtest_filter.

@muzarski muzarski marked this pull request as draft September 18, 2024 15:55
@muzarski muzarski marked this pull request as ready for review September 18, 2024 16:39
@muzarski muzarski self-assigned this Sep 18, 2024
@muzarski muzarski requested a review from dkropachev September 18, 2024 16:39
Currently, we rebuild (even if it exists) IT binary
if DONT_REBUILD_INTEGRATION_BIN env var is non-empty.
I believe it should be the opposite.

I changed it to something more clear, making use of `ifdef`.
Now, we:
- rebuild binary if `DONT_REBUILD_INTEGRATION_BIN` is not defined
- don't rebuild binary (if it exists) otherwise
Copy link
Collaborator

@dkropachev dkropachev left a comment

Choose a reason for hiding this comment

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

Please remove:

ifndef DONT_REBUILD_INTEGRATION_BIN
	DONT_REBUILD_INTEGRATION_BIN := ${EMPTY}
endif

And use:

ifdef DONT_REBUILD_INTEGRATION_BIN
run-test-integration-scylla: build-integration-test-bin-if-missing
else
run-test-integration-scylla: build-integration-test-bin
endif

@muzarski muzarski force-pushed the fix-build-it-condition-in-ci branch from c458fef to ed2ed65 Compare September 18, 2024 17:00
@muzarski
Copy link
Collaborator Author

CI passed, I'm merging

@muzarski muzarski merged commit caf12d9 into scylladb:master Sep 18, 2024
7 checks passed
@muzarski muzarski mentioned this pull request Oct 3, 2024
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.

2 participants