cget icon indicating copy to clipboard operation
cget copied to clipboard

Build cache

Open maddanio opened this issue 3 years ago • 5 comments

  • 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

maddanio avatar Jan 11 '21 10:01 maddanio

DeepCode's analysis on #cd6ce6 found:

  • :warning: 2 warnings, :information_source: 2 minor issues. :point_down:

Top issues

Description Example fixes
hashlib.sha1 is insecure. Consider changing it to a secure hashing algorithm (e.g. SHA256). Occurrences: :wrench: Example fixes
Missing close for zipfile.ZipFile, add close or use a with block. Occurrences: :wrench: 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: :wrench: Example fixes

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

ghost avatar Jan 11 '21 10:01 ghost

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

pfultz2 avatar Jan 21 '21 20:01 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. 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?

maddanio avatar Jan 26 '21 22:01 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).

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.

pfultz2 avatar Jan 27 '21 00:01 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).

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?

maddanio avatar Jan 27 '21 07:01 maddanio