Update should check version requirements using post-update values instead of what's currently installed#1120
Update should check version requirements using post-update values instead of what's currently installed#1120isc-jlechtne merged 5 commits intomainfrom
Conversation
…tead of what's currently installed
isc-jlechtne
requested review from
isc-cborbonm,
isc-dchui,
isc-egabhart,
isc-eneil,
isc-jili,
isc-kiyer,
isc-pbarton and
isc-tleavitt
as code owners
isc-dchui
left a comment
isc-dchui
left a comment
There was a problem hiding this comment.
First round of feedback!
isc-jili
left a comment
isc-jili
left a comment
There was a problem hiding this comment.
Left a couple comments but looks good @isc-jlechtne !
| // Iterate over sourceList; if the required version expression is equivalent to this one, add this module name to that version | ||
| set newVersion = 1 | ||
| for i=1:1:$listlength(sourceList) { | ||
| if $find($listget(sourceList, i), version) { |
There was a problem hiding this comment.
Is is possible that $find is too loose of a search? ie. would there be a case where the version incorrectly matches only a substring?
There was a problem hiding this comment.
The syntax of one element in sourceList looks like "module-a, module-b: ^1.0.0" for a case where both module-a and module-b require version ^1.0.0 of whatever module is being evaluated (the case here being adding some module-x to this string if it also requires ^1.0.0) so there won't be a case of this incorrectly passing since that would require a module name to have the identical version string in it.
Could alternatively do $piece($listget(sourceList, i), " ", *) = version as a way to completely isolate the version expression but splitting up the string on the delimiter should be unnecessary for this case.
|
I am realizing actually that even though we framed this issue as "update command" related, GetRequiredVersionExpression() is called by many more functionalities than just update. So I have 2 questions:
|
For 2, there's a sample module in PR #1050 you can copy over. |
4 participants
Description
Addresses issue #1119
Source of the fix is centered around the
%IPM.Utils.Module::GetRequiredVersionExpression()method. It uses a SQL call to determine what versions are installed and what relies on them for checking version requirements. It also accepts a list of modules to exclude from this check. This fix adds a dependency graph to that list. When given a dependency graph, the method will now add the modules from the exclusion list to add them manually after the SQL call. This addresses the update case since the dependency graph will have the required version expressions associated with what update will install as opposed to what's currently installed.Testing
PR includes a new integration test to the update suite to check for this case.