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.

Sign up for GitHub

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

Jump to bottom

fix(datasource/custom): do not run digest update on version updates #29730

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+40 −1
Open

fix(datasource/custom): do not run digest update on version updates #29730

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e8a31d
custom datasources: do not run digest update on version updates
marcoesters Jun 14, 2024
747c333
Merge branch 'main' into custom-datasource-digest-updates
rarkins Aug 19, 2024
dff209b
Add unit test for single-entry custom datasource releases
marcoesters Aug 22, 2024
87ba1b3
Use boolean and if-else chain for digest update logic
marcoesters Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions lib/workers/repository/process/lookup/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4004,6 +4004,37 @@ describe('workers/repository/process/lookup/index', () => {
]);
});

it("handles version update with new digest for single-entry custom datasource releases", async () => {
config.currentValue = "1.0.0";
config.packageName = "my-package";
config.datasource = "custom.package";
config.currentDigest = 'zzzzzzzzzzzzzzz';
getCustomDatasourceReleases.mockResolvedValueOnce({
releases: [
{
version: '1.0.1',
newDigest: '0123456789abcdef',
},
],
});
const { updates, warnings } = await Result.wrap(
lookup.lookupUpdates(config),
).unwrapOrThrow();
expect(updates).toEqual([
{
bucket: 'non-major',
newDigest: '0123456789abcdef',
newMajor: 1,
newMinor: 0,
newPatch: 1,
newValue: '1.0.1',
newVersion: '1.0.1',
updateType: 'patch',
},
]);
expect(warnings).toEqual([]);
});

it('handles digest update for non-version', async () => {
config.currentValue = 'alpine';
config.packageName = 'node';
Expand Down
9 changes: 8 additions & 1 deletion lib/workers/repository/process/lookup/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,14 @@ export async function lookupUpdates(
// Add digests if necessary
if (supportsDigests(config.datasource)) {
if (config.currentDigest) {
if (!config.digestOneAndOnly || !res.updates.length) {
// Custom datasources should not run a digest update
// on the current version if it already runs a version update.
if (
!(
config.digestOneAndOnly ?? config.datasource.startsWith('custom.')
) ||
!res.updates.length
) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to convert this logic to an if/else to reduce the amount of negating. The if path can have logger.trace() inside it and the else then will contain the res.updates.push()

Copy link
Collaborator

Choose a reason for hiding this comment

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

(for future readability - the proposed change has definitely crossed a threshold for readability, maybe already had)

Copy link
Author

Choose a reason for hiding this comment

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

I tried a few logic combinations and they are all very much unreadable, so I resorted to a boolean that will hopefully add some more to the semantics.

This is, unfortunately, still all very hacky, but I think a more sustainable solution will require deeper insights into the renovate code bases and use cases than I currently possess.

// digest update
res.updates.push({
updateType: 'digest',
Expand Down