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

Fix crash when same pack is located on 2 paths #1063

Merged

Conversation

Torbjorn-Svensson
Copy link
Collaborator

@Torbjorn-Svensson Torbjorn-Svensson commented Jul 12, 2023

Scenario:
csolution.yml:

packs:
  - pack: ARM::[email protected]
    path: ./ARM.CMSIS.5.9.0/

cproject.yml:

packs:
  - pack: [email protected]

In the above scenario, the same pack is defined on 2 different paths. The one in the csolution.yml is a project local path, and for the cproject.yml, it can be found in the $CMSIS_PACK_ROOT.

As the RteModel does not allow "duplicates", the RtePackage pointer, for the second pdsc file to be loaded, will be released when calling RteModel->Validate() and thus, only instances that is known by the model should be contained in m_loadedPacks in the ProjMgrWorker instance.

Contributed by STMicroelectronics

@github-actions
Copy link

github-actions bot commented Jul 12, 2023

Test Results

    3 files    15 suites   2m 4s ⏱️
261 tests 261 ✔️ 0 💤 0
783 runs  783 ✔️ 0 💤 0

Results for commit 3eda6d6.

♻️ This comment has been updated with latest results.

@jkrech jkrech requested review from edriouk and brondani July 13, 2023 05:32
Copy link
Collaborator

@brondani brondani left a comment

Choose a reason for hiding this comment

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

@Torbjorn-Svensson Thanks for this finding and contribution.
Please see my suggestion below and also add or extended a test case to cover the new code.

tools/projmgr/src/ProjMgrKernel.cpp Outdated Show resolved Hide resolved
Scenario:
csolution.yml:
packs:
  - pack: ARM::[email protected]
    path: ./ARM.CMSIS.5.9.0/

cproject.yml:
packs:
  - pack: [email protected]

In the above scenario, the same pack is defined on 2 different paths.
The one in the csolution.yml is a project local path, and for the
cproject.yml, it can be found in the $CMSIS_PACK_ROOT.

As the RteModel does not allow "duplicates", the RtePackage pointer,
for the second pdsc file to be loaded, will be released when
calling RteModel->Validate() and thus, only instances that is known
by the model should be contained in m_loadedPacks in the ProjMgrWorker
instance.

Contributed by STMicroelectronics

Signed-off-by: Torbjörn SVENSSON <[email protected]>
Co-authored-by: Daniel Brondani <[email protected]>
@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Merging #1063 (c9918a4) into main (9fd7498) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head c9918a4 differs from pull request most recent head 3eda6d6. Consider uploading reports for the commit 3eda6d6 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1063      +/-   ##
==========================================
- Coverage   54.61%   54.61%   -0.01%     
==========================================
  Files         116      116              
  Lines       22703    22701       -2     
  Branches    12607    12608       +1     
==========================================
- Hits        12400    12398       -2     
  Misses       8148     8148              
  Partials     2155     2155              
Flag Coverage Δ
projmgr-cov 82.47% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
tools/projmgr/src/ProjMgrKernel.cpp 86.25% <100.00%> (+0.35%) ⬆️

... and 1 file with indirect coverage changes

Copy link
Collaborator

@brondani brondani left a comment

Choose a reason for hiding this comment

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

@edriouk edriouk merged commit 6164c95 into Open-CMSIS-Pack:main Jul 14, 2023
31 of 32 checks passed
@Torbjorn-Svensson Torbjorn-Svensson deleted the pr/fix-crash-when-same-pack branch July 14, 2023 09:40
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