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

[projmgr] Add --frozen-packs argument to csolution #1225

Merged

Conversation

Torbjorn-Svensson
Copy link
Collaborator

@Torbjorn-Svensson Torbjorn-Svensson commented Nov 24, 2023

The --frozen-packs option is only applicable for the following commands:

  • convert
  • update-rte

If cbuild-pack.yml needs to be created or updated, then --frozen-pack will treat this as an error and fail the command.

Contributed by STMicroelectronics

Copy link

github-actions bot commented Nov 24, 2023

Test Results

    7 files  ±0    53 suites  ±0   4m 59s ⏱️ +22s
189 tests ±0  172 ✔️ ±0  17 💤 ±0  0 ±0 
704 runs  ±0  636 ✔️ ±0  68 💤 ±0  0 ±0 

Results for commit a39244d. ± Comparison against base commit ab8b01d.

♻️ This comment has been updated with latest results.

@jkrech jkrech requested a review from brondani November 27, 2023 07:57
@jkrech
Copy link
Member

jkrech commented Nov 27, 2023

@Torbjorn-Svensson, is this PR ready for review or in Draft?
Do you think this should/must be included in CMSIS-Toolbox 2.2.0?

@Torbjorn-Svensson
Copy link
Collaborator Author

@Torbjorn-Svensson, is this PR ready for review or in Draft? Do you think this should/must be included in CMSIS-Toolbox 2.2.0?

As we discusses on the meetings the previous 2 weeks, this was the followup PR that should preferably be included in the 2.2.0 release, but was "optional" in case time did not allow to make it stable.
The only thing that is in "draft" mode of this PR is the naming. I hope that there will be some feedback on the PR if the name --frozen-packs is acceptable or if there is something else that would fit the need better.

@jkrech
Copy link
Member

jkrech commented Nov 27, 2023

Alternative names for the option I can think of are:

--no-update-packs

similar to rte file update:

--no-update-rte
--fixed-packs

@jkrech jkrech closed this Nov 27, 2023
@jkrech jkrech reopened this Nov 27, 2023
@Torbjorn-Svensson
Copy link
Collaborator Author

Alternative names for the option I can think of are:

--no-update-packs

similar to rte file update:

--no-update-rte
--fixed-packs

I do not have any strict opinion on what it should be named, but I think it would be best if it's not confused with what --no-update-rte is supposed to do.

@edriouk
Copy link
Collaborator

edriouk commented Dec 11, 2023

This PR can make use of "Dedicated method to Update RTE folder" #1244

@jkrech
Copy link
Member

jkrech commented Dec 12, 2023

In the technical meeting from 2023-12-05 we agreed to use the command line option:

--frozen-packs

@Torbjorn-Svensson
Copy link
Collaborator Author

@edriouk Can you please take a look at the 2nd commit in this PR as it appears to fix the issue that I talked about in the meeting today.

Copy link
Collaborator

@edriouk edriouk 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, I need more information about the change in RteProject.cpp.
To my understanding, --frozen-packs argument is used against already converted projects all config files are already copied to the RTE directory and all @base files are created.
Could you explain the case you are trying to fix?
@brondani, could you please approve the rest.

@Torbjorn-Svensson Torbjorn-Svensson force-pushed the pr/cbuild-pack-frozen branch 2 times, most recently from 07e4169 to 3c48651 Compare December 13, 2023 12:15
Torbjorn-Svensson and others added 2 commits December 14, 2023 14:40
When resolving the base file version for RTE files when the RTE directory
has not yet been populated, fall back to the version defined in the pack.

Contributed by STMicroelectronics

Signed-off-by: Torbjörn SVENSSON <[email protected]>
Co-authored-by: Samuel HULTGREN <[email protected]>
The --frozen-packs option is only applicable for the following commands:
- convert
- update-rte

If cbuild-pack.yml needs to be created or updated, then --frozen-pack will
treat this as an error and fail the command.

Contributed by STMicroelectronics

Signed-off-by: Torbjörn SVENSSON <[email protected]>
@Torbjorn-Svensson
Copy link
Collaborator Author

Rebased on projmgr/2.2.1. There should be no more changes needed before this PR can be merged.
Please have a look at this and let me know if there is anything more I need to fix.

Copy link

codecov bot commented Dec 14, 2023

Codecov Report

Merging #1225 (a39244d) into main (ab8b01d) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1225      +/-   ##
==========================================
+ Coverage   59.20%   59.22%   +0.02%     
==========================================
  Files         117      117              
  Lines       23564    23578      +14     
  Branches    13106    13116      +10     
==========================================
+ Hits        13951    13965      +14     
  Misses       7348     7348              
  Partials     2265     2265              
Flag Coverage Δ
buildmgr-cov 77.91% <ø> (ø)
packchk-cov 64.16% <ø> (ø)
projmgr-cov 84.33% <100.00%> (+0.03%) ⬆️

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

Files Coverage Δ
tools/projmgr/src/ProjMgr.cpp 77.33% <100.00%> (+0.35%) ⬆️
tools/projmgr/src/ProjMgrParser.cpp 100.00% <100.00%> (ø)
tools/projmgr/src/ProjMgrYamlEmitter.cpp 95.33% <100.00%> (+0.02%) ⬆️
tools/projmgr/src/ProjMgrYamlParser.cpp 77.12% <100.00%> (+0.09%) ⬆️

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 the contribution.
LGTM

@brondani brondani merged commit 0f761bc into Open-CMSIS-Pack:main Dec 14, 2023
84 checks passed
@Torbjorn-Svensson Torbjorn-Svensson deleted the pr/cbuild-pack-frozen branch December 14, 2023 21:38
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.

4 participants