feat: debian datasource #13463

Ka0o0 · 2022-01-10T12:41:40Z

Changes:

Adds a Debian package datasource (Closes Add apt datasource for private repositories #7041)

Context:

This datasource is able to search multiple Debian package sources that provide a Packages file as described here.
This repository standard defines components within a release repository (e.g. main within bullseye release).

The datasource works by downloading the compressed Packages file for each component and extracting it. Currently, only .xz files are supported, but it should be possible to add other compression standards as well.
The file is downloaded using curl, which is a shell cmd call, into a configurable folder.
The file is only downloaded if the local version is older than what the server has (using curl's -z parameter) and is only extracted if needed.

The file is then searched through, currently, line by line which is a bit slow with Node.JS and further work might improve the performance by removing unused information from the extracted files.
It then returns the latest version which is, differently from what repology is returning us, the version you can specify with apt install, like apt-get install curl=7.74.0-1.3+deb11u1. An example PR created with this banch merged looks like this: Ka0o0/renovate-testrepo#2

Documentation (please check one with an [x])

I have updated the documentation, or
No documentation update is required

How I've tested my work (please tick one)

I have verified these changes via:

Code inspection only, or
Newly added/modified unit tests, or
No unit tests but ran on a real repository, or
Both unit tests + ran on a real repository

Open Points

Where to add documentation?
Is the current implementation regarding the datasource configuration valid like it is currently?
This datasource calls some third party tools which are currently available in the renovate Docker image. Are any changes to how this is used required, like explicitly stating the dependency somewhere?

HonkingGoose · 2022-01-10T13:04:09Z

Where to add documentation?

Once the code for the feature is approved by the maintainers, the documentation can be added in a new lib/datasource/deb/readme.md file, I think.

The "Repology datasource" has docs that you can look at for inspiration. ¹

https://github.com/renovatebot/renovate/blob/main/lib/datasource/repology/readme.md ↩

rarkins · 2022-01-10T13:37:16Z

Hi @Ka0o0, thanks for this PR! Can you create a feature request issue to link to from this PR, and so we can discuss the requirements/implementation proposal there first?

Ka0o0 · 2022-01-10T14:26:46Z

Hi @rarkins,

thanks for pointing that out. Actually, there is #7041 already opened which I've referenced in this PR's description.

lib/datasource/deb/index.spec.ts

lib/datasource/deb/index.ts

lib/datasource/api.ts

lib/datasource/deb/index.ts

viceice · 2022-01-13T16:18:41Z

Please do not use simple Done like comments, they produce a lot of noise.

@HonkingGoose Can we extend our contribution guide? So people should simply resolve conversations when done and only use re-request review button. Any only if no maintainer answer, use comments after ~5days.

@rarkins WDYT?

HonkingGoose · 2022-01-13T16:28:09Z

@HonkingGoose Can we extend our contribution guide? So people should simply resolve conversations when done and only use re-request review button. Any only if no maintainer answer, use comments after ~5days.

Sounds good to me. Is the goal here to reduce noise, and to "teach" contributors to wait a bit before commenting?

Our .github/contributing.md file says: ¹

Re-requesting a review

Please do not ping your reviewer(s) by mentioning them in a new comment.
Instead, use the re-request review functionality.
Read more about this in the GitHub docs, Re-requesting a review.

https://github.com/renovatebot/renovate/blob/main/.github/contributing.md#re-requesting-a-review ↩

viceice · 2022-01-13T16:31:38Z

@HonkingGoose Can we extend our contribution guide? So people should simply resolve conversations when done and only use re-request review button. Any only if no maintainer answer, use comments after ~5days.

Sounds good to me. Is the goal here to reduce noise, and to "teach" contributors to wait a bit before commenting?

So we only need some sentence about to suppress short comments like Done and instead resolve conversation.

lib/datasource/deb/index.spec.ts

lib/datasource/deb/index.ts

viceice · 2022-01-14T08:37:00Z

lib/datasource/deb/index.ts

+    return needsToDownload;
+  }
+
+  async probeExtractedPackage(


This is a goot candidate to add cache decorator, sample:

renovate/lib/manager/terraform/lockfile/hash.ts

Lines 61 to 69 in 4f65b57

@cache({

namespace: `datasource-${TerraformProviderDatasource.id}-build-hashes`,

key: (build: TerraformBuild) => build.url,

ttlMinutes: TerraformProviderHash.hashCacheTTL,

})

static async calculateSingleHash(

build: TerraformBuild,

cacheDir: string

): Promise<string> {

Mhm but what if the server returns a new compressed file, can I manually invalidate the cache entry then?

viceice

@rarkins I'm still somehow unconfident with the current cache mechanism. I think we should extract all updates and write to renovate cache with additional metadata. And only use cache if still valid.

viceice · 2022-02-18T13:11:29Z

lib/datasource/deb/index.spec.ts

+    const testPackagesFile = __dirname + '/test-data/Packages.gz';
+    const extractedTestFile = __dirname + '/test-data/Packages';


We have helpers for this, checkout fixtures class

The only test case where I think a fixture would make sense is this one. However, I don't quite know how to get it running.

If I understood it correctly, the Fixtures.mock method will register some temp files based on what is provided as a parameter and intercept FS calls to those files? So I'm setting up the test case like this (basically replacing line 44 - 45):

const fixtures: DirectoryJSON = {}; fixtures[extractedPackageFile] = Fixtures.get('Packages'); Fixtures.mock(fixtures);

But now the test case would fail here. Shouldn't the Fixtures' mock make sure that the file exists?

lib/datasource/deb/index.spec.ts

viceice · 2022-02-18T13:15:23Z

lib/datasource/deb/index.spec.ts

+      expect(res.releases).toHaveLength(1);
+
+      // validate that the server was called correctly
+      expect(httpMock.getTrace()).toHaveLength(1);


Suggested change

expect(httpMock.getTrace()).toHaveLength(1);

This will be always equal to your mocked http requests, so no benefit. 😉

Unfulfilled mocks are only evaluated after the test block. In the next line the array access on getTrace() would therefore be executed which would lead to an runtime exception in a failing test case scenario.

Yes and no, if you look at the http mock code, you can see a afterEach which will validate this, so we don't have to repeat it here. 😉

lib/datasource/deb/index.spec.ts

lib/datasource/deb/index.ts

viceice · 2022-02-18T13:27:30Z

lib/datasource/deb/index.ts

+    const rl = readline.createInterface({
+      input: fs.createReadStream(extractedFile),
+      terminal: false,
+    });


I think we should cache the compressed file and directly stream if possible. Would sava a lot disk space. 🤔

I agree that would indeed some disk space for the cost of computational expense. I was also thinking about removing some unused meta information from the extracted Package file which would also reduce the package file size and reduce the lookup time (which is more important I guess). What do you think?

lib/util/fs/index.ts

Co-authored-by: Michael Kriese <[email protected]>

rarkins · 2022-03-25T10:14:26Z

Needs test fixes

viceice · 2022-04-29T03:01:36Z

index.mjs

@@ -0,0 +1,41 @@
+import { createReadStream } from 'fs';


why this file?

viceice · 2022-04-29T03:02:42Z

lib/util/fs/index.ts

+  return fs.createReadStream(path);
+}
+
+export function access(path: string, mode?: number): Promise<void> {


move to ./proxies.ts

viceice · 2022-04-29T03:04:01Z

lib/datasource/deb/index.spec.ts

@@ -0,0 +1,182 @@
+import { copyFile, mkdirp, stat } from 'fs-extra';


file structure changed, please move datasource to right folder.

please don't force-push

rarkins · 2022-10-19T04:51:38Z

Closing due to staleness

Add Debian package datasource

bc79714

rarkins changed the title ~~Add Debian package datasource~~ feat: debian datasource Jan 10, 2022

Ka0o0 mentioned this pull request Jan 10, 2022

Add apt datasource for private repositories #7041

Closed

viceice requested changes Jan 10, 2022

View reviewed changes

HonkingGoose reviewed Jan 11, 2022

View reviewed changes

lib/datasource/api.ts Outdated Show resolved Hide resolved

Ka0o0 added 5 commits January 13, 2022 13:58

use http class

1c5a116

fix caching

270e363

use different url format

3c2f433

switch to gz compression using Node.JS zlib

5fe458c

cleanup

0cf2328

Ka0o0 commented Jan 13, 2022

View reviewed changes

lib/datasource/deb/index.ts Show resolved Hide resolved

Ka0o0 requested a review from viceice January 13, 2022 15:56

viceice reviewed Jan 13, 2022

View reviewed changes

HonkingGoose mentioned this pull request Jan 13, 2022

docs(contributing): clarify pull request review guidelines #13546

Merged

6 tasks

Ka0o0 added 4 commits January 14, 2022 07:22

cleanup

8faee2d

more cleanup

bd88abb

add comment

3206401

use custom fs layer

1de78f1

Ka0o0 requested a review from viceice January 14, 2022 08:22

viceice reviewed Jan 14, 2022

View reviewed changes

Ka0o0 added 4 commits January 14, 2022 09:06

remove cache dir config

314f709

fix tests, make binary arch configurable per repo link

096c1f3

cleanup

c79f809

allow returning multiple releases per package

a8e7a32

Ka0o0 added 9 commits February 14, 2022 07:15

Merge remote-tracking branch 'main' into feat/deb-datasource

ea50609

Add test for invalid server response

0bcb6ff

refactor: extract extraction of componentUrl from registryUrl

e8f1468

change test to cover two components case

615f8a1

add test describing parsing of registry url

56d4964

add test for behavior if different metadata across components

8b8f6da

add test suite/release synonym

389aa60

update comment

c439a68

add deb datasource in correct order

283055e

HonkingGoose requested a review from viceice February 14, 2022 10:54

viceice reviewed Feb 18, 2022

View reviewed changes

viceice and others added 10 commits February 18, 2022 14:32

Merge branch 'main' into feat/deb-datasource

6caa868

enable cache

d95a3bb

refactor logging

27196a0

remove duplicate test

ba1a006

use tmp-promise

483bc2a

fix local path creation

331a39f

make some methods static

90b3d72

fix optional access

30913ea

Co-authored-by: Michael Kriese <[email protected]>

remove unused import

c7dd40a

update contributor list

dcae944

Ka0o0 requested a review from viceice February 21, 2022 15:12

Merge branch 'main' into feat/deb-datasource

193ae88

rarkins marked this pull request as draft March 25, 2022 10:13

Merge branch 'main' into feat/deb-datasource

f01b8ba

viceice reviewed Apr 29, 2022

View reviewed changes

rarkins closed this Oct 19, 2022

github-actions bot locked as resolved and limited conversation to collaborators Nov 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: debian datasource #13463

feat: debian datasource #13463

Ka0o0 commented Jan 10, 2022 •

edited

Loading

HonkingGoose commented Jan 10, 2022

rarkins commented Jan 10, 2022

Ka0o0 commented Jan 10, 2022

viceice commented Jan 13, 2022

HonkingGoose commented Jan 13, 2022

Re-requesting a review

viceice commented Jan 13, 2022

viceice Jan 14, 2022

Ka0o0 Jan 17, 2022

viceice left a comment

viceice Feb 18, 2022

Ka0o0 Feb 21, 2022

viceice Feb 18, 2022

Ka0o0 Feb 21, 2022

viceice Feb 21, 2022

viceice Feb 18, 2022

Ka0o0 Feb 21, 2022

rarkins commented Mar 25, 2022

viceice Apr 29, 2022

viceice Apr 29, 2022

viceice Apr 29, 2022

rarkins commented Oct 19, 2022

	@cache({
	namespace: `datasource-${TerraformProviderDatasource.id}-build-hashes`,
	key: (build: TerraformBuild) => build.url,
	ttlMinutes: TerraformProviderHash.hashCacheTTL,
	})
	static async calculateSingleHash(
	build: TerraformBuild,
	cacheDir: string
	): Promise<string> {

		const testPackagesFile = __dirname + '/test-data/Packages.gz';
		const extractedTestFile = __dirname + '/test-data/Packages';

		@@ -0,0 +1,182 @@
		import { copyFile, mkdirp, stat } from 'fs-extra';

feat: debian datasource #13463

feat: debian datasource #13463

Conversation

Ka0o0 commented Jan 10, 2022 • edited Loading

Changes:

Context:

Documentation (please check one with an [x])

How I've tested my work (please tick one)

Open Points

HonkingGoose commented Jan 10, 2022

Footnotes

rarkins commented Jan 10, 2022

Ka0o0 commented Jan 10, 2022

viceice commented Jan 13, 2022

HonkingGoose commented Jan 13, 2022

Re-requesting a review

Footnotes

viceice commented Jan 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viceice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rarkins commented Mar 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rarkins commented Oct 19, 2022

Ka0o0 commented Jan 10, 2022 •

edited

Loading