skip reinstillation of twoliter from source #3479
Conversation
When we are testing changes to Twoliter, we need to install from git. In that case the sha is not going to match the version output from Twoliter --version. This means that we have to rebuild and install Twoliter each time we call cargo make. It takes 10 minutes to compile Twoliter. Fix this such that TWOLITER_ALLOW_BINARY_INSTALL=false skips checking the version when any `twoliter` is installed.
| if [ "${allow_bin}" != "true" ]; then | ||
| echo "Twoliter binary found and --allow-binary-install is false. Skipping install." | ||
| exit 0 |
There was a problem hiding this comment.
I don't agree with this change and would like to see it reverted.
The version check is pretty important as otherwise there's no guarantee that the build will happen with the pinned version rather than whatever happens to be present. This potentially affects the correctness of any build where the binary is not available and the source build must be used, which can happen for other reasons than a developer working locally on twoliter.
It might be better for the twoliter version to not include the Git SHA, or to allow the path to the binary to be overridden (e.g. to ~/twoliter/target/release/twoliter) and skip the version check if the override is set - with a loud warning that the override is preventing the version check.
There was a problem hiding this comment.
In that case the sha is not going to match the version output from Twoliter --version.
❯ ./tools/twoliter/twoliter -V
twoliter 0.0.3
I don't understand what sha we are or aren't matching since twoliter -V doesn't print a SHA.
Issue number:
Closes #3478
Description of changes:
When we are testing changes to Twoliter, we need to install from git. In that case the sha is not going to match the version output from Twoliter --version. This means that we have to rebuild and install Twoliter each time we call cargo make. It takes 10 minutes to compile Twoliter.
Fix this such that TWOLITER_ALLOW_BINARY_INSTALL=false skips checking the version when any
twoliteris installed.Testing done:
Used this locally and it fixed the problem.
Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.