Take config's OS, Dist, OS X image name, known Env var values into cache slug computation

[BanzaiMan]

This PR addresses the issue with cache name collisions by taking into account extra pieces of information known at the build script compilation time.

Related issues:

travis-ci/travis-ci#4393
travis-ci/travis-ci#3023
travis-ci/travis-ci#5460

[BanzaiMan]

Currently, this PR does not take secure env vars into consideration. As we are just taking the SHA256 digest, I think it is acceptable to take secure env vars into computation. (There is no way to recover secure env vars from the SHA256 digest.)

[BanzaiMan]

Actually, I think it is best to not include secure environment variables, since we do not use salt for the digest.

[BanzaiMan]

Or add salt on the app side.

[BanzaiMan]

Example:

https://staging.travis-ci.org/BanzaiMan/travis_staging_test/jobs/492870#L161-L163

[BanzaiMan]

The salt can be defined user or repo level in the database. Either as a new field (preferred) or reusing an existing secret (less preferred).

[BanzaiMan]

Maybe we should take the osx_image value into account. This is a bit more granular information than OS X release (sw_vers -productVersion), but it is better than corrupt caches.

[BanzaiMan]

We should also consider not printing the hash at all, or omitting some characters. This requires a change to casher.

[theuni]

Would it be possible to include an extra user-defined salt value as well, so that individual builds may set themselves apart from others in the matrix?

We're currently working around this with a hack: https://github.com/bitcoin/bitcoin/blob/master/.travis.yml#L38

It would be great to use instead, something like:

    - cache_salt: arm
      env: HOST=arm-linux-gnueabihf PACKAGES="g++-arm-linux-gnueabihf" DEP_OPTS="NO_QT=1" CHECK_DOC=1 GOAL="install" BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports"
    - cache_salt: win32
...

Edit: or does this take all yml-defined env vars into account as well? If so, that would work solve our problem as well :)

[theuni]

Ok, I clicked too fast, assuming this only applied to vars set outside of the .yml. Obviously that's not the case, and this work addresses our use-case exactly. Thanks!

[theuni]

There are some cases where user-defined variables contain pre-defined vars from the builder. For example: https://travis-ci.org/bitcoin/bitcoin/jobs/125738866#L161

Would the values of vars like TRAVIS_BUILD_ID/TRAVIS_COMMIT/etc. be expanded before being used to calculate the slug? If so, I imagine there are many that would cause new cache id's for each build.

[BanzaiMan]

@theuni Thanks for taking time to look at this PR. I really appreciate the interest in this feature.

This PR considers only the environment variables defined in .travis.yml because only these are known at the build script compile time. This characteristic is important because in order to fetch and store caches securely, URLs have to be signed; and the signing _must happen_ at the script's compile time. (See the description of this PR.)

Some environment variables are evaluated and assigned only at the run time. Some can be reasoned beforehand (e.g., TRAVIS_BUILD_ID) but are probably not fit to be a part of the key for the cache (because the hash thus computed will never match again for future builds). We simply need to stay away from them. Users may assign others (e.g., in a script), but there is no way for travis-build to know them.

I also thought about some mechanism for the user to influence the cache name besides setting environment variables (e.g., with cache_salt as proposed in a previous comment here) with a key in .travis.yml. I like the idea, but I think the cost is rather high (in terms of cognition, and in terms of implementation). To achieve the same effect, however, one can simply set an extra environment variable that does nothing but signify the name of the cache (e.g., CACHE_NAME=arm).

[BanzaiMan]

My inclinations:

• Leave secure environments out of the mix
• Omit a few characters in the digest name in the logs

[theuni]

@BanzaiMan Agree on leaving out secure vars.

As a suggestion though, it would be helpful to see (maybe behind a debug var?) which env vars have been considered for the slug, without their values.

[BanzaiMan]

@theuni Do you want that information always printed? Sees a bit noisy. And adding a key to .travis.yml seems kludgy.

[theuni]

@BanzaiMan No, only for troubleshooting the non-obvious cases. As a stupid example off the top of my head:

before:

env:
  global:
    - FOO=bar
matrix:
  include:
    - compiler: gcc
      env: BAR=blah
    - compiler: clang
      env:

after:

env:
  global:
    - FOO=bar BAR=blah
matrix:
  include:
    - compiler: gcc
      env:
    - compiler: clang
      env: BAR=

Do those end up with new cache ids?

It would be helpful in this case to see what's happening with BAR, as I'm not sure if empty is the same as nonexistent.

Not the end of the world if printing it is a hassle though, It would only take a few quick experiments to figure out what's happening.

[BanzaiMan]

The very presence of such a variable should affect the cache name. BAR= and (absence ofBAR) are different.

[BanzaiMan]

This has been deployed to production. I will observe how it behaves for the next several hours, after which, if it's behaving well, I will merge this code and redeploy.