Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

topology1: build the production topologies one level down #5071

Merged
merged 1 commit into from
Dec 13, 2021

Conversation

marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Dec 10, 2021

Move generated *.conf and *.tplg v1 files down from:

  build_tools/topology/topology1/*.{conf,tplg}

_to:

  build_tools/topology/topology1/production/*.{conf,tplg}

... then copy/"install" the production/* subdirectory two levels up.

This fixes the race condition #5067 that also copied a random number of
development/ and dsp_enhancement/ topologies, sometimes even truncating
these.

In other words, this commit REMOVES the following two copies:

build_tools/topology/development/       # randomly corrupted copy, removed
build_tools/topology/dsp_enhancement/   # randomly corrupted copy, removed

build_tools/topology/topology1/development/    # real build dir, unchanged
build_tools/topology/topology1/dsp_enhancement # real build dir, unchanged

Production topologies are copied only to help with the v1->v2
transition. That duplication makes the build directory confusing enough,
no need to extend that copy to development topologies. A single instance
of development topologies in the build directory is enough.

This removal may break some CI script(s): this is perfect because such
CI script(s) were copying randomly corrupted data.

Signed-off-by: Marc Herbert marc.herbert@intel.com

Move generated *.conf and *.tplg v1 files down from:

  build_tools/topology/topology1/*.{conf,tplg}

_to:

  build_tools/topology/topology1/production/*.{conf,tplg}

... then copy/"install" the production/* subdirectory two levels up.

This fixes the race condition thesofproject#5067 that also copied a random number of
development/ and dsp_enhancement/ topologies, sometimes even truncating
these.

In other words, this commit REMOVES the following two copies:

build_tools/topology/development/       # randomly corrupted copy, removed
build_tools/topology/dsp_enhancement/   # randomly corrupted copy, removed

build_tools/topology/topology1/development/    # real build dir, unchanged
build_tools/topology/topology1/dsp_enhancement # real build dir, unchanged

Production topologies are copied only to help with the v1->v2
transition. That duplication makes the build directory confusing enough,
no need to extend that copy to development topologies. A single instance
of development topologies in the build directory is enough.

This removal may break some CI script(s): this is perfect because such
CI script(s) were copying randomly corrupted data.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
Copy link
Collaborator Author

marc-hb commented Dec 10, 2021

sof-ci/jenkins is down this weekend, will have to SOFCI TEST on Monday.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Dec 13, 2021

Unrelated alsabat failure in https://sof-ci.01.org/sofpr/PR5071/build11351/devicetest/?model=BYT_MB_NOCODEC&testcase=check-alsabat-headset-playback, BDW not available for testing, everything else green.

@lgirdwood lgirdwood merged commit 07c63c1 into thesofproject:main Dec 13, 2021
@marc-hb marc-hb deleted the fix-topo-race branch December 13, 2021 23:42
@aiChaoSONG
Copy link
Collaborator

The sof-tgl-nocodec-ci.tplg topology is in the development folder, but the folder is not moved two level up as the production folder, so the test use this topology failed in the daily. the issue is spotted by @miRoox

@marc-hb
Copy link
Collaborator Author

marc-hb commented Dec 14, 2021

From the commit message:

This removal may break some CI script(s): this is perfect because such
CI script(s) were copying randomly corrupted data.

Corruption was in only the most extreme, most unlucky case. However the number of development topologies copied up was definitely random. In other words, sof-tgl-nocodec-ci.tplg could be missing sometimes.

The CI script must be updated to fetch the original sof-tgl-nocodec-ci.tplg one level down instead of the randomly copied duplicate.

@aiChaoSONG
Copy link
Collaborator

The CI script must be updated to fetch the original sof-tgl-nocodec-ci.tplg one level down instead of the randomly copied duplicate.

@miRoox @XiaoyunWu6666 This may impact the build service, FYI

@marc-hb
Copy link
Collaborator Author

marc-hb commented Dec 14, 2021

Another option is to "promote" sof-tgl-nocodec-ci.m4 to production status by moving it one level up, that should fix the issue without any CI change. In #5067 @plbossart wrote:

The intent is that they [development topologies] are developer-maintained, but not suitable for release or use by non-developers.

Should validation rely on such development/ files? It's a grey area but I would say no. I think validation should validate production files.

@miRoox
Copy link

miRoox commented Dec 14, 2021

However, as the name shows, sof-tgl-nocodec-ci is developed for CI 🤣

@XiaoyunWu6666
Copy link
Contributor

XiaoyunWu6666 commented Dec 14, 2021

@miRoox @XiaoyunWu6666 This may impact the build service, FYI

Shall we separate two sub folders for production topologies and development topologies ? something like below
image

where tplg/production stores the topologies of build_tools/topology/topology1/production/sof-.tplg
and tplg/development stores the topologies of build_tools/topology/topology1/development/sof-
.tplg

@marc-hb
Copy link
Collaborator Author

marc-hb commented Dec 14, 2021

However, as the name shows, sof-tgl-nocodec-ci is developed for CI

Ideally, validation should validate what products use. In cases where validation must diverge and validate something no one uses because "reasons", then such divergence should at least not be hidden in some obscure "development/" subdirectory. It should at least be as high and as visible as what people use and found in the same place.

tl;dr: sof-tgl-nocodec-ci should be promoted and moved away from other unmaintained and unsupported topologies.

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.

6 participants