Skip to content

Fix calculation of $(1)_DEP_PKGS_SHA in Makefile.cache#10764

Merged
saiarcot895 merged 1 commit intosonic-net:masterfrom
saiarcot895:caching-dependencies-fix
May 9, 2022
Merged

Fix calculation of $(1)_DEP_PKGS_SHA in Makefile.cache#10764
saiarcot895 merged 1 commit intosonic-net:masterfrom
saiarcot895:caching-dependencies-fix

Conversation

@saiarcot895
Copy link
Copy Markdown
Contributor

@saiarcot895 saiarcot895 commented May 6, 2022

Signed-off-by: Saikrishna Arcot [email protected]

Why I did it

In Makefile.cache, for $(1)_DEP_PKGS_SHA, the intention is to include
the DEP_MOD_SHA and MOD_HASH of each of the current package's
dependencies. However, there's a level of dereferencing missing; instead
of grabbing the value of $(dfile)_DEP_MOD_SHA, it is literally using the
variable name $(dfile)_DEP_MOD_SHA. This means that the value of this
variable will not change when some dependency changes.

The impact of this is in indirect dependencies. For a specific
example, if there is some change in sairedis, then sairedis will be
rebuilt (because there's a change within that component), and swss will
be rebuilt (because it's a direct dependency), but
docker-swss-layer-buster will not get rebuilt, because only the direct
dependencies are effectively being checked, and those aren't changing.

How I did it

Make sure the value of $(1)_MOD_DEP_PKGS, $(dfile)_DEP_MOD_SHA, and $(dfile)_MOD_HASH are used, not the literal text.

Before:

FLAGS: target/debs/buster/libsnmp-base_5.7.3+dfsg-5_all.deb.flags, DEP:
DEP: target/debs/buster/libsnmp-base_5.7.3+dfsg-5_all.deb.dep, MOD:Makefile.cache
libsnmp-base_5.7.3+dfsg-5_all.deb_MOD_DEP_PKGS: libnl-3-200_3.5.0-1_amd64.deb  libnl-3-200_3.5.0-1_amd64.deb
libsnmp-base_5.7.3+dfsg-5_all.deb_DEP_MOD_SHA_FILES: target/debs/buster/libnl-3-200_3.5.0-1_amd64.deb.flags target/debs/buster/libnl-3-200_3.5.0-1_amd64.deb.dep.sha   target/debs/buster/libnl-3-200_3.5.0-1_amd64.deb.flags target/debs/buster/libnl-3-200_3.5.0-1_amd64.deb.dep.sha
libsnmp-base_5.7.3+dfsg-5_all.deb_DEP_PKGS_SHA: libsnmp-base_5.7.3+dfsg-5_all.deb_MOD_DEP_PKGS_DEP_MOD_SHA libsnmp-base_5.7.3+dfsg-5_all.deb_MOD_DEP_PKGS_MOD_HASH
libsnmp-base_5.7.3+dfsg-5_all.deb_MOD_DEP_FILES: target/debs/buster/libsnmp-base_5.7.3+dfsg-5_all.deb.flags target/debs/buster/libsnmp-base_5.7.3+dfsg-5_all.deb.dep.sha
libsnmp-base_5.7.3+dfsg-5_all.deb_MODE_CACHE_FILE := libsnmp-base_5.7.3+dfsg-5_all.deb-555bd7e832cdb7993b180d0-5ec7ff76937dcabf40f58bd.tgz
FLAGS: target/debs/buster/libyang1_1.0.184-2_amd64.deb.flags, DEP:
DEP: target/debs/buster/libyang1_1.0.184-2_amd64.deb.dep, MOD:Makefile.cache
libyang1_1.0.184-2_amd64.deb_MOD_DEP_PKGS:
libyang1_1.0.184-2_amd64.deb_DEP_MOD_SHA_FILES:
libyang1_1.0.184-2_amd64.deb_DEP_PKGS_SHA: libyang1_1.0.184-2_amd64.deb_MOD_DEP_PKGS_DEP_MOD_SHA libyang1_1.0.184-2_amd64.deb_MOD_DEP_PKGS_MOD_HASH
libyang1_1.0.184-2_amd64.deb_MOD_DEP_FILES: target/debs/buster/libyang1_1.0.184-2_amd64.deb.flags target/debs/buster/libyang1_1.0.184-2_amd64.deb.dep.sha
libyang1_1.0.184-2_amd64.deb_MODE_CACHE_FILE := libyang1_1.0.184-2_amd64.deb-7ce90aa69332df946e0b075-44189ebcfc77adbf0294393.tgz

After:

FLAGS: target/debs/buster/libsnmp-base_5.7.3+dfsg-5_all.deb.flags, DEP:
DEP: target/debs/buster/libsnmp-base_5.7.3+dfsg-5_all.deb.dep, MOD:Makefile.cache
libsnmp-base_5.7.3+dfsg-5_all.deb_MOD_DEP_PKGS: libnl-3-200_3.5.0-1_amd64.deb  libnl-3-200_3.5.0-1_amd64.deb
libsnmp-base_5.7.3+dfsg-5_all.deb_DEP_MOD_SHA_FILES: target/debs/buster/libnl-3-200_3.5.0-1_amd64.deb.flags target/debs/buster/libnl-3-200_3.5.0-1_amd64.deb.dep.sha   target/debs/buster/libnl-3-200_3.5.0-1_amd64.deb.flags target/debs/buster/libnl-3-200_3.5.0-1_amd64.deb.dep.sha
libsnmp-base_5.7.3+dfsg-5_all.deb_DEP_PKGS_SHA: adc83b19e793491b1c6ea0f da9b9d27a4105e6b83efe49 adc83b19e793491b1c6ea0f da9b9d27a4105e6b83efe49
libsnmp-base_5.7.3+dfsg-5_all.deb_DEP_MOD_SHA: a9ab5ade419cadaca31289c
libsnmp-base_5.7.3+dfsg-5_all.deb_MOD_DEP_FILES: target/debs/buster/libsnmp-base_5.7.3+dfsg-5_all.deb.flags target/debs/buster/libsnmp-base_5.7.3+dfsg-5_all.deb.dep.sha
libsnmp-base_5.7.3+dfsg-5_all.deb_MODE_CACHE_FILE := libsnmp-base_5.7.3+dfsg-5_all.deb-a9ab5ade419cadaca31289c-bffe759ac3036e4bc1f26c0.tgz
FLAGS: target/debs/buster/libyang1_1.0.184-2_amd64.deb.flags, DEP:
DEP: target/debs/buster/libyang1_1.0.184-2_amd64.deb.dep, MOD:Makefile.cache
libyang1_1.0.184-2_amd64.deb_MOD_DEP_PKGS:
libyang1_1.0.184-2_amd64.deb_DEP_MOD_SHA_FILES:
libyang1_1.0.184-2_amd64.deb_DEP_PKGS_SHA:
libyang1_1.0.184-2_amd64.deb_DEP_MOD_SHA: adc83b19e793491b1c6ea0f
libyang1_1.0.184-2_amd64.deb_MOD_DEP_FILES: target/debs/buster/libyang1_1.0.184-2_amd64.deb.flags target/debs/buster/libyang1_1.0.184-2_amd64.deb.dep.sha
libyang1_1.0.184-2_amd64.deb_MODE_CACHE_FILE := libyang1_1.0.184-2_amd64.deb-adc83b19e793491b1c6ea0f-3403cc0cfda553ecdbb02d6.tgz

How to verify it

Change the git hash used for the src/sonic-sairedis/SAI submodule, and verify that docker-swss-layer-buster gets rebuilt.

Note that this also means that PR build times may increase, since indirect dependencies are now properly being checked.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

In Makefile.cache, for $(1)_DEP_PKGS_SHA, the intention is to include
the DEP_MOD_SHA and MOD_HASH of each of the current package's
dependencies. However, there's a level of dereferencing missing; instead
of grabbing the value of $(dfile)_DEP_MOD_SHA, it is literally using the
variable name $(dfile)_DEP_MOD_SHA. This means that the value of this
variable will not change when some dependency changes.

The impact of this is in transitive dependencies. For a specific
example, if there is some change in sairedis, then sairedis will be
rebuilt (because there's a change within that component), and swss will
be rebuilt (because it's a direct dependency), but
docker-swss-layer-buster will not get rebuilt, because only the direct
dependencies are effectively being checked, and those aren't changing.

Signed-off-by: Saikrishna Arcot <[email protected]>
@saiarcot895 saiarcot895 merged commit dc6f325 into sonic-net:master May 9, 2022
@saiarcot895 saiarcot895 deleted the caching-dependencies-fix branch May 9, 2022 19:06
qiluo-msft pushed a commit that referenced this pull request May 10, 2022
In Makefile.cache, for $(1)_DEP_PKGS_SHA, the intention is to include
the DEP_MOD_SHA and MOD_HASH of each of the current package's
dependencies. However, there's a level of dereferencing missing; instead
of grabbing the value of $(dfile)_DEP_MOD_SHA, it is literally using the
variable name $(dfile)_DEP_MOD_SHA. This means that the value of this
variable will not change when some dependency changes.

The impact of this is in transitive dependencies. For a specific
example, if there is some change in sairedis, then sairedis will be
rebuilt (because there's a change within that component), and swss will
be rebuilt (because it's a direct dependency), but
docker-swss-layer-buster will not get rebuilt, because only the direct
dependencies are effectively being checked, and those aren't changing.

Signed-off-by: Saikrishna Arcot <[email protected]>
judyjoseph pushed a commit that referenced this pull request May 16, 2022
In Makefile.cache, for $(1)_DEP_PKGS_SHA, the intention is to include
the DEP_MOD_SHA and MOD_HASH of each of the current package's
dependencies. However, there's a level of dereferencing missing; instead
of grabbing the value of $(dfile)_DEP_MOD_SHA, it is literally using the
variable name $(dfile)_DEP_MOD_SHA. This means that the value of this
variable will not change when some dependency changes.

The impact of this is in transitive dependencies. For a specific
example, if there is some change in sairedis, then sairedis will be
rebuilt (because there's a change within that component), and swss will
be rebuilt (because it's a direct dependency), but
docker-swss-layer-buster will not get rebuilt, because only the direct
dependencies are effectively being checked, and those aren't changing.

Signed-off-by: Saikrishna Arcot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants