[Makefile.cache] fix an issue that non-direct dependencies are not ac… #8965
Conversation
…counted in component hash calculation Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
stepanblyschak
requested review from
lguohan,
qiluo-msft and
xumia
as code owners
|
@xumia Could you help review? |
|
Your problem statement is specific to docker-syncd-mlnx-rpc. Is it something missing in the file |
|
@qiluo-msft I believe all vendors RPC images will have similar issue. In general, if there is a component X that depends on component Y that depends on component Z. Changing Z's version does not result in a hash value change for component X. |
| @@ -182,7 +182,10 @@ define GET_MOD_DEP_SHA | |||
| $(eval $(1)_DEP_FILES_MISSING := $(filter-out $(wildcard $($(1)_DEP_MOD_SHA_FILES)),$($(1)_DEP_MOD_SHA_FILES)) ) | |||
| $(if $($(1)_DEP_FILES_MISSING), $(warning "[ DPKG ] Dependecy file(s) are not found for $(1) : $($(1)_DEP_FILES_MISSING))) | |||
|
|
|||
| $(eval $(1)_DEP_MOD_SHA := $(shell git hash-object $($(1)_DEP_MOD_SHA_FILES) \ | |||
| # Include package dependencies hash values into package hash calculation | |||
| $(eval $(1)_DEP_PKGS_SHA := $(foreach dfile,$(1)_MOD_DEP_PKGS,$(dfile)_DEP_MOD_SHA $(dfile)_MOD_HASH)) | |||
There was a problem hiding this comment.
It has impact on the cache efficiency when building the new version of the cache artifacts, and the mod dependency tree become useless.
Currently, the nightly build will update the cache. Thinking about a scenario, there is no change in a branch, but when building the new image, the package binary will be changed in each build. It means that all the dependency packages will be rebuilt, it is not as we expected.
For instance, B depended on A, and C depended on B:
A <- B <- C
When built A, and the cache refreshed, whatever code change or not for the files of A, we need to build B and C, that is why we do not want to depend on the package hash.
We use the source files dependency today, for package A, we need to list out all the files A depended. A will generate a hash file, B will depend on file. And C will indirectly depend on the file.
There was a problem hiding this comment.
@xumia Could you please clarify your understanding of "package hash"? It seems like you are talking about a package binary hash. This is not what this PR introduces.
What this PR does is, for example (A <- B <- C), for package C DEP_MOD_SHA value hash calculation it includes B's and A's DEP_MOD_SHA and MOD_HASH values. DEP_MOD_SHA and MOD_HASH are not package binary hash values, but a hash calculated based on makefiles, source code and build flags. If that does not change neither for B, C or A hash values remain the same. If the A's sources change, then, of course that affects B's hash value and with this change affects C's hash value. So that A, B and C are all rebuilt.
"And C will indirectly depend on the file." - This is what I want to do as I don't see it is done by cache system. If that is true maybe I missed something, could you please point me to the code that does it?
There was a problem hiding this comment.
@stepanblyschak , sorry for the misunderstanding, it is nothing to do with the package binary, and thanks for your detail explain, you are fixing the recursive dependencies issue, right?
Currently, the cache file consists of _DEP_MOD_SHA and _MOD_HASH, only considering two levels dependencies, it is not enough, right? The _DEP_MOD_SHA should contain the recursive dependencies info, right?
$(eval $(1)_MOD_CACHE_FILE := $(1)-$($(1)_DEP_MOD_SHA)-$($(1)_MOD_HASH).tgz)
|
Not necessary to include in 201911, the dpkg cache feature is not supported in the branch |
…counted in component hash calculation (#8965) #### Why I did it Fixed an issue that changing SDK version leads to cache framework taking cached syncd RPC image rather then rebuilding syncd RPC based on new syncd with new SDK. Investigation showed that cache framework calculates a component hash based on direct dependencies. Syncd RPC image hash consists of two parts: one is the flags of syncd RPC (platform, ENABLE_SYNCD_RPC) and syncd RPC direct dependencies makefiles. None of the syncd RPC direct dependencies are modified when SDK version changes, so hash is unchanged. #### How I did it To fix this issue, include the hash of dependencies into current component hash calculation, e.g.: In calcultation of the hash ```docker-syncd-mlnx-rpc.gz-274dfed3f52f2effa9989fc-39344350436f9b06d28b470.tgz```, the hash of syncd is included: ```docker-syncd-mlnx.gz-48ee88ac54b201e0e107b15-7bbea320025177a2121e440.tgz``` in which the hash of SDK is included. #### How to verify it Build with cache enabled and check that changing SDK version leads to a different hash of syncd rpc image: SDK version 4.5.1002: ``` docker-syncd-mlnx.gz-48ee88ac54b201e0e107b15-7bbea320025177a2121e440.tgz docker-syncd-mlnx-rpc.gz-274dfed3f52f2effa9989fc-39344350436f9b06d28b470.tgz ``` SDK version 4.5.1002-005: ``` docker-syncd-mlnx.gz-18baf952e3e0eda7cda7c3c-e5668f4784390d5dffd55af.tgz docker-syncd-mlnx-rpc.gz-4a6e59580eda110b5709449-552f76be135deaf750aeab2.tgz ```
…counted in component hash calculation (#8965) #### Why I did it Fixed an issue that changing SDK version leads to cache framework taking cached syncd RPC image rather then rebuilding syncd RPC based on new syncd with new SDK. Investigation showed that cache framework calculates a component hash based on direct dependencies. Syncd RPC image hash consists of two parts: one is the flags of syncd RPC (platform, ENABLE_SYNCD_RPC) and syncd RPC direct dependencies makefiles. None of the syncd RPC direct dependencies are modified when SDK version changes, so hash is unchanged. #### How I did it To fix this issue, include the hash of dependencies into current component hash calculation, e.g.: In calcultation of the hash ```docker-syncd-mlnx-rpc.gz-274dfed3f52f2effa9989fc-39344350436f9b06d28b470.tgz```, the hash of syncd is included: ```docker-syncd-mlnx.gz-48ee88ac54b201e0e107b15-7bbea320025177a2121e440.tgz``` in which the hash of SDK is included. #### How to verify it Build with cache enabled and check that changing SDK version leads to a different hash of syncd rpc image: SDK version 4.5.1002: ``` docker-syncd-mlnx.gz-48ee88ac54b201e0e107b15-7bbea320025177a2121e440.tgz docker-syncd-mlnx-rpc.gz-274dfed3f52f2effa9989fc-39344350436f9b06d28b470.tgz ``` SDK version 4.5.1002-005: ``` docker-syncd-mlnx.gz-18baf952e3e0eda7cda7c3c-e5668f4784390d5dffd55af.tgz docker-syncd-mlnx-rpc.gz-4a6e59580eda110b5709449-552f76be135deaf750aeab2.tgz ```
…counted in component hash calculation
Signed-off-by: Stepan Blyschak stepanb@nvidia.com
Why I did it
Fixed an issue that changing SDK version leads to cache framework taking cached syncd RPC image rather then rebuilding syncd RPC based on new syncd with new SDK.
Investigation showed that cache framework calculates a component hash based on direct dependencies. Syncd RPC image hash consists of two parts: one is the flags of syncd RPC (platform, ENABLE_SYNCD_RPC) and syncd RPC direct dependencies makefiles. None of the syncd RPC direct dependencies are modified when SDK version changes, so hash is unchanged.
How I did it
To fix this issue, include the hash of dependencies into current component hash calculation, e.g.:
In calcultation of the hash
docker-syncd-mlnx-rpc.gz-274dfed3f52f2effa9989fc-39344350436f9b06d28b470.tgz, the hash of syncd is included:docker-syncd-mlnx.gz-48ee88ac54b201e0e107b15-7bbea320025177a2121e440.tgzin which the hash of SDK is included.How to verify it
Build with cache enabled and check that changing SDK version leads to a different hash of syncd rpc image:
SDK version 4.5.1002:
SDK version 4.5.1002-005:
Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)