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

Preserve requirements.psd1 and use it for managed dependencies snapshot comparison #670

Merged
merged 1 commit into from
Aug 31, 2021

Conversation

AnatoliB
Copy link
Contributor

@AnatoliB AnatoliB commented Aug 20, 2021

Potentially fixes #647 without the need to introduce a breaking change.

  • When a new snapshot is created, copy the requirements.psd1 file from the app content to the snapshot directory, so that each snapshot is explicitly associated with the app author intent.
  • When the worker evaluates an existing snapshot content to decide whether this snapshot is acceptable or not:
    • Compare the requirements.psd1 files in the snapshot and in the app content before looking for specific versions.
    • If the requirements.psd1 content matches, consider this snapshot acceptable (since this snapshot is the result of applying Save-Module, regardless of the module version matching rules implemented internally by Save-Module).
    • If these files don't match (or the snapshot does not contain requirements.psd1 because this is an older snapshot, created before rolling out this fix), proceed with looking for specific module versions as usual.

As a result, for example, if requirements.psd1 contains 'PnP.PowerShell' = '1.5', invoking Save-Module will still install version 1.5.0. But, since the original requirements.psd1 is preserved with the snapshot, the next PowerShell Worker instance will consider this snapshot acceptable and will start using it right away, without creating another one. The problem is solved without introducing a breaking change.

@AnatoliB AnatoliB marked this pull request as ready for review August 23, 2021 22:47
Copy link
Contributor

@Francisco-Gamino Francisco-Gamino left a comment

Choose a reason for hiding this comment

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

LGTM. We should add an E2E test case, which can be added in a separate PR.

@AnatoliB
Copy link
Contributor Author

Tested on a private site extension, looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants