Build cache

[maddanio]

• optionally cache builds based on package hash
• the package hash is generated from the recipe and recursively dependencies
• currently only respects dependencies from recipes and not such from the package itself
• also introduces a file lock in the cache to avoid undefined behavior with concurrent cget runs
• introduces --recipe-deps-only option to ignore dependencies inside source, which greatly speeds up cached installations

[ghost]

DeepCode's analysis on #cd6ce6 found:

• ⚠️ 2 warnings, ℹ️ 2 minor issues. 👇

Top issues

Description	Example fixes
hashlib.sha1 is insecure. Consider changing it to a secure hashing algorithm (e.g. SHA256). Occurrences: package.py:40 prefix.py:232	🔧 Example fixes
Missing close for zipfile.ZipFile, add close or use a with block. Occurrences: util.py:121 util.py:313	🔧 Example fixes
standard import "import os, shutil, shlex, six, inspect, click, contextlib, uuid, sys, functools, hashlib" should be placed before "import os, shutil, shlex, six, inspect, click, contextlib, uuid, sys, functools, hashlib" Occurrences: prefix.py:1 package.py:1	🔧 Example fixes

👉 View analysis in DeepCode’s Dashboard | Configure the bot

[pfultz2]

We probably need to include the toolchain settings in the hash. We could hash the cget/cget.cmake file but we also need to hash any files included in cget.cmake(like when the user is using cget init -t my-toolchain.cmake.

[maddanio]

well, for me thats not so relevant right now, but yeah, for completeness sure. i was leaving some slack to get it working at all, and in doubt just clear the cache. btw, we will also here run into the problem that sub-dependencies inherit their parents cmake defines, which is unclean

[pfultz2]

Also, there are CI error. Are you able to look into those?

[maddanio]

turns out it is not as simple. the way I make it work in my fork is by isolating packages against each other. i.e. i remove all references to the build local cget dir in the toolchain file and instead pass a list of CMAKE_PREFIX_PATH to the build. in the next step to make this more robust I should also fix the define inheritance, i.e. that dependents inherit the defines of the packages they where built from. you want me to pull those bits in here?

[pfultz2]

turns out it is not as simple. the way I make it work in my fork is by isolating packages against each other. i.e. i remove all references to the build local cget dir in the toolchain file and instead pass a list of CMAKE_PREFIX_PATH to the build.

That should probably only happen when the build-cache is enabled(otherwise it should fallback on the old behavior).

next step to make this more robust I should also fix the define inheritance, i.e. that dependents inherit the defines of the packages they where built from. you want me to pull those bits in here?

There should be a different flag for defines that dont inherit as this is a breaking change. Furthermore, the hash should use the inherited defines to compute the hash.

[maddanio]

turns out it is not as simple. the way I make it work in my fork is by isolating packages against each other. i.e. i remove all references to the build local cget dir in the toolchain file and instead pass a list of CMAKE_PREFIX_PATH to the build.

That should probably only happen when the build-cache is enabled(otherwise it should fallback on the old behavior).

Not actually sure. What is the advantage of the merged prefix? Using specific prefixes should always work afaics.

next step to make this more robust I should also fix the define inheritance, i.e. that dependents inherit the defines of the packages they where built from. you want me to pull those bits in here?

There should be a different flag for defines that dont inherit as this is a breaking change. Furthermore, the hash should use the inherited defines to compute the hash.

So the inheriting was intended? It seemed to me that it was incidental because it effectively means the order of requirements will change the result. Also usually defines made in one packet are not meant for others?