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

feat(manager): Support poetry custom repositories. #4524

Merged
merged 11 commits into from
Sep 30, 2019
Merged

feat(manager): Support poetry custom repositories. #4524

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7b81ece
feat(manager): Support poetry custom repositories.
gilbsgilbs Sep 21, 2019
324301a
Poetry: add tests for empty and missing registries.
gilbsgilbs Sep 21, 2019
93c072d
Poetry: keep querying pypi.org even if we find other registries.
gilbsgilbs Sep 21, 2019
0256ad0
Poetry: let datasource set default registry when possible.
gilbsgilbs Sep 21, 2019
817157e
Poetry: Put pypi index first.
gilbsgilbs Sep 21, 2019
9de3f2b
Poetry: safely get source section from pyproject file.
gilbsgilbs Sep 22, 2019
3d0db58
Poetry: return `null` instead of empty array when source section is e…
gilbsgilbs Sep 22, 2019
a3a36c2
Poetry: prettify.
gilbsgilbs Sep 22, 2019
f8f47e6
Poetry: fix tests.
gilbsgilbs Sep 22, 2019
dd7892d
Merge branch 'master' into poetry-reprositories
gilbsgilbs Sep 29, 2019
14452cf
Poetry: deduplicate registry urls.
gilbsgilbs Sep 29, 2019
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
20 changes: 19 additions & 1 deletion lib/manager/poetry/extract.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ export function extractPackageFile(
if (!deps.length) {
return null;
}
return { deps };

return { deps, registryUrls: extractRegistries(pyprojectfile) };
}

function extractFromSection(
Expand Down Expand Up @@ -84,3 +85,20 @@ function extractFromSection(
});
return deps;
}

function extractRegistries(pyprojectfile: PoetryFile): string[] {
const sources = pyprojectfile.tool.poetry.source;
gilbsgilbs marked this conversation as resolved.
Show resolved Hide resolved
const registryUrls: string[] = ['https://pypi.org/pypi/'];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually we prefer to handle defaults within datasource, not manager. Otherwise it's too hard to override the default registry URL in Renovate config.

Copy link
Contributor Author

@gilbsgilbs gilbsgilbs Sep 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Do you think 0256ad0 is enough? Unless I'm missing something, I don't think I can easily use the default registry from the pypi datasource if I have other registries to add. Hence, I've done something similar to what pip_requirements manager does.


if (!Array.isArray(sources)) {
return registryUrls;
}

for (const source of sources) {
if (source.url) {
registryUrls.push(source.url);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we de-dupe this in case the user configures pypi.org explicitly?

Copy link
Contributor Author

@gilbsgilbs gilbsgilbs Sep 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, I misread this comment as a statement instead of a question. I committed code that dedupes, but I don't think it is necessary as pypi datasource will only append pypi.org if registryUrls is empty. If poetry manager adds pypi.org itself, pypi datasource won't add any other datasource. Let me revert this.

Edit: unless you meant “configures pypi.org explicitly” in pyproject.toml, which I think is a bit silly 🤔 . Anyways, it is not a bad thing to dedup I think. Let's not revert after all.

}
}

return registryUrls;
}
6 changes: 6 additions & 0 deletions lib/manager/poetry/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export interface PoetrySection {
dependencies: Record<string, PoetryDependency | string>;
'dev-dependencies': Record<string, PoetryDependency | string>;
extras: Record<string, PoetryDependency | string>;
source?: PoetrySource[];
}

export interface PoetryFile {
Expand All @@ -15,3 +16,8 @@ export interface PoetryDependency {
git?: string;
version?: string;
}

export interface PoetrySource {
name?: string;
url?: string;
}
20 changes: 20 additions & 0 deletions test/manager/poetry/__snapshots__/extract.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`lib/manager/poetry/extract extractPackageFile() can parse empty registries 1`] = `
Array [
"https://pypi.org/pypi/",
]
`;

exports[`lib/manager/poetry/extract extractPackageFile() can parse missing registries 1`] = `
Array [
"https://pypi.org/pypi/",
]
`;

exports[`lib/manager/poetry/extract extractPackageFile() extracts multiple dependencies (with dep = {version = "1.2.3"} case) 1`] = `
Array [
Object {
Expand Down Expand Up @@ -157,6 +169,14 @@ Array [
]
`;

exports[`lib/manager/poetry/extract extractPackageFile() extracts registries 1`] = `
Array [
"https://pypi.org/pypi/",
"https://foo.bar/simple/",
"https://bar.baz/+simple/",
]
`;

exports[`lib/manager/poetry/extract extractPackageFile() handles multiple constraint dependencies 1`] = `
Array [
Object {
Expand Down
16 changes: 16 additions & 0 deletions test/manager/poetry/_fixtures/pyproject.6.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[tool.poetry]
name = "example 6"
version = "0.1.0"
description = ""
authors = ["John Doe <[email protected]>"]

[tool.poetry.dependencies]
dep0 = "0.0.0"

[[tool.poetry.source]]
name = "foo"
url = "https://foo.bar/simple/"

[[tool.poetry.source]]
name = "bar"
url = "https://bar.baz/+simple/"
9 changes: 9 additions & 0 deletions test/manager/poetry/_fixtures/pyproject.7.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[tool.poetry]
name = "example 7"
version = "0.1.0"
description = ""
authors = ["John Doe <[email protected]>"]
source = []

[tool.poetry.dependencies]
dep0 = "0.0.0"
25 changes: 25 additions & 0 deletions test/manager/poetry/extract.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ const pyproject5toml = readFileSync(
'utf8'
);

const pyproject6toml = readFileSync(
'test/manager/poetry/_fixtures/pyproject.6.toml',
'utf8'
);

const pyproject7toml = readFileSync(
'test/manager/poetry/_fixtures/pyproject.7.toml',
'utf8'
);

describe('lib/manager/poetry/extract', () => {
describe('extractPackageFile()', () => {
let config;
Expand Down Expand Up @@ -57,6 +67,21 @@ describe('lib/manager/poetry/extract', () => {
expect(res.deps).toMatchSnapshot();
expect(res.deps).toHaveLength(1);
});
it('extracts registries', () => {
const res = extractPackageFile(pyproject6toml, config);
expect(res.registryUrls).toMatchSnapshot();
expect(res.registryUrls).toHaveLength(3);
});
it('can parse empty registries', () => {
const res = extractPackageFile(pyproject7toml, config);
expect(res.registryUrls).toMatchSnapshot();
expect(res.registryUrls).toHaveLength(1);
});
it('can parse missing registries', () => {
const res = extractPackageFile(pyproject1toml, config);
expect(res.registryUrls).toMatchSnapshot();
expect(res.registryUrls).toHaveLength(1);
});
it('skips git dependencies', () => {
const content =
'[tool.poetry.dependencies]\r\nflask = {git = "https://github.com/pallets/flask.git"}\r\nwerkzeug = ">=0.14"';
Expand Down