Abort on indeterminstic checkout

[jkloetzke]

No description provided.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.95%. Comparing base (64dd214) to head (86c6b1a).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #623   +/-   ##
=======================================
  Coverage   88.94%   88.95%           
=======================================
  Files          48       48           
  Lines       15616    15618    +2     
=======================================
+ Hits        13890    13893    +3     
+ Misses       1726     1725    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rhubert]

Thanks. I'm wondering if there should be a command line argument or a policy to disable this abort? Otherwise it might break existing builds as the warning might not have been recognized before.

[jkloetzke]

You can always restart the build manually. So this is not a permanent error and you should still be able to build projects that trigger this error. Or do you mean that we should suppress even that and optionally support the old behaviour?

[rhubert]

We have a (known broken but god given) CI Setup. Bob build runs in a container without persistent storage and with not enough resources for a complete build. This only works because we have the cache - so we can (automatically) retry the build several times....
I fear that with this change we might end up in a situation where we hit the nondeterministic package on each retry. This is not a problem for HEAD since we could apply a patch to fix the package. But for bugfix-releases additional fixes just to make the CI happy again are near to impossible.

[jkloetzke]

I see. Thanks for the explanation. Then a new policy is probably needed, I guess...

[rhubert]

🤔 for our current releases we can also use an older bob version without the abort.
For further releases we just need to be sure there is no indeterministic checkout. Something like a --verfiy-checkout build doing the checkout and compare the result to the expected result hash from the buildid file?

[jkloetzke]

🤔 for our current releases we can also use an older bob version without the abort.

Usually, this is something that we should avoid. Taken to it's logical end, we would not even need policies. Just use the right Bob version for the project. But this is a usability nightmare. My goal is that older projects work with newer Bob versions. We might break something by accident but that should happen rarely.

For further releases we just need to be sure there is no indeterministic checkout. Something like a --verfiy-checkout build doing the checkout and compare the result to the expected result hash from the buildid file?

That could turn out to be tricky. We fully allow the user to tinker with the sources. The check that I patched here is only running when the sources have been checkout out initially, which is the only safe state for such a check. But you will only ever notice the problem when artifacts were uploaded.

To make it even worse, even if we would introduce something like --verify-checkout that runs the checkout twice, we would probably not detect problems. The autoconf class will not call autoconf again unless configure.ac was changed. So you might not even detect the problem until you delete the source workspace and re-run it. :-/ Tricky...