Skip to content

Introduce gcovpreload repo to support code coverage report generation for swss and sairedis - #11323

Open
pettershao-ragilenetworks wants to merge 36 commits into
sonic-net:masterfrom
pettershao-ragilenetworks:introduce-gcovpreload-repo
Open

pettershao-ragilenetworks wants to merge 36 commits into
sonic-net:masterfrom
pettershao-ragilenetworks:introduce-gcovpreload-repo

Conversation

@pettershao-ragilenetworks

Copy link
Copy Markdown
Contributor

Why I did it

This new component will be compiled as a dynamic library, then it can be used as a dependency for those components being required to generate code coverage report. This repo is migrated from sonic-sairedis repo and treated as a new repo in sonic-buildimage. The reason for that please check sonic-net/sonic-sairedis#972

How I did it

make gcovpreload as a new repo in sonic-buildimage and compile it to a dynamic library and a regular deb

How to verify it

After #972 merged, check whether sairedis can normally generate coverage report

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

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

Description for the changelog

Introduce gcovpreload repo to support code coverage report generation for swss and sairedis

Link to config_db schema for YANG module changes

NA

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

@linux-foundation-easycla

linux-foundation-easycla Bot commented Aug 5, 2022 •

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

@qiluo-msft

Copy link
Copy Markdown
Collaborator

@xumia Could you review?

@pettershao-ragilenetworks

Copy link
Copy Markdown
Contributor Author

@qiluo-msft @lguohan @xumia Can you help check this pipeline compling issues? I didn't modify any files during compiling process, don't know why it is failing.

Signed-off-by: “pettershao” <pettershao@ragilenetworks.com>
Add LIBGCOV_PRELOAD dependents in DOCKER_SONIC_VS_DEPENDS
Comment thread dockers/docker-orchagent/docker-init.j2 Outdated
fi

exec /usr/local/bin/supervisord
echo /usr/lib/libgcovpreload.so > /etc/ld.so.preload

@xumia xumia Sep 7, 2022 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding in the debian postinst step?
The package should be only for code coverage, not necessary to install it in product, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding in the debian postinst step? ------updated
The package should be only for code coverage, not necessary to install it in product, right? ----yes, this package is only for code coverage.

Comment thread .gitignore Outdated
Comment on lines +47 to +52
# Subdirectories in src
src/preload/gcovpreload/debian/*
!src/preload/gcovpreload/debian/changelog
!src/preload/gcovpreload/debian/control
!src/preload/gcovpreload/debian/rules

@xumia xumia Sep 7, 2022 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding in src/preload/.gitignore?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

change .gitignore from /.gitignore to /src/preload/..gitignore
change /.gitignore to /src/preload/.gitignore
Add libgcovpreload.postinst
Update .gitignore
use postinst to replace "echo /usr/lib/libgcovpreload.so > /etc/ld.so.preload"
use postinst to replace "echo /usr/lib/libgcovpreload.so > /etc/ld.so.preload"
use postinst to replace "echo /usr/lib/libgcovpreload.so > /etc/ld.so.preload"
use postinst to replace "echo /usr/lib/libgcovpreload.so > /etc/ld.so.preload"
To trigger rebuild&test
@pettershao-ragilenetworks
pettershao-ragilenetworks requested review from xumia and removed request for lguohan and qiluo-msft September 19, 2022 09:33
Comment thread rules/docker-orchagent.mk
$(DOCKER_ORCHAGENT)_DBG_DEPENDS += $(SWSS_DBG) \
$(LIBSWSSCOMMON_DBG) \
$(LIBSAIREDIS_DBG)
$(LIBSAIREDIS_DBG) \

@xumia xumia Sep 21, 2022 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the change is not necessary, may need to remove the last character "\", or miss a dependent target name.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have the same concern.

Comment thread rules/sairedis.mk
$(LIBSAIREDIS)_VERSION = $(LIBSAIREDIS_VERSION)
$(LIBSAIREDIS)_NAME = $(LIBSAIREDIS_NAME)
$(LIBSAIREDIS)_DEPENDS += $(LIBSWSSCOMMON_DEV)
$(LIBSAIREDIS)_DEPENDS += $(LIBSWSSCOMMON_DEV) $(LIBGCOV_PRELOAD)

@xumia xumia Sep 21, 2022 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure my understanding is correct. Is it for code coverage? Is it possible to install it on demand, we can enable it in PR build, for official build to install in devices, looks like it is not necessary. We may need to clean some tools for build/test only in the image.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, I have raised a sairedis PR to support GCOV, but need preload lib(the reason why can't build with sairedis repo together has mentioned in PR: sonic-net/sonic-sairedis#972). As I know, pipeline framework will download vs docker and install sairedis deb into it, and then do testing. So i need to make sure that the downloaded docker encompass preload lib.
Is it possible to install it on demand -> I think a MACRO "ENABLE_GCOV" defined in rules/config will be fine. I will add it soon, default value set to 'y'.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not want LIBGCOV_PRELOAD in the sonic image. You only need it in test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there other ways to solve this problem? to make sure that vs docker downloaded by pipeline framework when testing encompass gcov preload lib and this lib will not in sonic image ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not want LIBGCOV_PRELOAD in the sonic image. You only need it in test.

Do you have any idea about how to solve this problem?

@xumia
xumia requested a review from qiluo-msft September 21, 2022 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants