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/uv): set registry URLs #31186

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(manager/uv): set registry URLs #31186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aae863d
refactor(manager/pep621): extract custom managers
mkniewallner Sep 3, 2024
e4b3bd7
feat(manager/uv): set registry URLs
mkniewallner Sep 3, 2024
56d1e68
refactor(manager/uv): only set registries if necessary
mkniewallner Sep 4, 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
55 changes: 55 additions & 0 deletions lib/modules/manager/pep621/processors/uv.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,61 @@ describe('modules/manager/pep621/processors/uv', () => {
},
]);
});

it('uses default PyPI and extra URLs when setting extra-index-url', () => {
const pyproject = {
tool: {
uv: {
'extra-index-url': [
'https://foo.example.com',
'https://bar.example.com',
],
},
},
};
const dependencies = [{ packageName: 'dep1' }];

const result = processor.process(pyproject, dependencies);

expect(result).toEqual([
{
packageName: 'dep1',
registryUrls: [
'https://foo.example.com',
'https://bar.example.com',
'https://pypi.org/pypi/',
],
},
]);
});

it('uses index and extra URLs when setting index-url and extra-index-url', () => {
const pyproject = {
tool: {
uv: {
'index-url': 'https://foobar.example.com',
'extra-index-url': [
'https://foo.example.com',
'https://bar.example.com',
],
},
},
};
const dependencies = [{ packageName: 'dep1' }];

const result = processor.process(pyproject, dependencies);

expect(result).toEqual([
{
packageName: 'dep1',
registryUrls: [
'https://foo.example.com',
'https://bar.example.com',
'https://foobar.example.com',
],
},
]);
});
});

describe('updateArtifacts()', () => {
Expand Down
24 changes: 23 additions & 1 deletion lib/modules/manager/pep621/processors/uv.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ import { exec } from '../../../../util/exec';
import type { ExecOptions, ToolConstraint } from '../../../../util/exec/types';
import { getSiblingFileName, readLocalFile } from '../../../../util/fs';
import { Result } from '../../../../util/result';
import { PypiDatasource } from '../../../datasource/pypi';
import type {
PackageDependency,
UpdateArtifact,
UpdateArtifactsResult,
Upgrade,
} from '../../types';
import { type PyProject, UvLockfileSchema } from '../schema';
import { type PyProject, type Uv, UvLockfileSchema } from '../schema';
import { depTypes, parseDependencyList } from '../utils';
import type { PyProjectProcessor } from './types';

Expand All @@ -32,6 +33,13 @@ export class UvProcessor implements PyProjectProcessor {
),
);

const registryUrls = extractRegistryUrls(uv);
if (registryUrls.length) {
for (const dep of deps) {
dep.registryUrls = [...registryUrls];
}
Comment on lines +38 to +40
Copy link
Collaborator

Choose a reason for hiding this comment

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

Normally if registry URLs apply to the entire package file then we want them set once at the packageFile level instead duplicated into every dep.

Also, are these likely to all be pypi datasource now and forever? Or is it possible that there could be a git dependency mixed in there at one point (which would then make the registry URL invalid for that dep)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Normally if registry URLs apply to the entire package file then we want them set once at the packageFile level instead duplicated into every dep.

There is a plan to be able to map specific dependencies to specific sources in astral-sh/uv#171, similarly to what PDM does I believe. According to this comment it should be worked on soon enough. It doesn't mean that we can't implement what you're suggesting, which makes sense, but maybe since this is meant to evolve, setting registries at the dependency level is more conservative?

Also, are these likely to all be pypi datasource now and forever? Or is it possible that there could be a git dependency mixed in there at one point (which would then make the registry URL invalid for that dep)

That's a really good point, even today it's already possible to set other sources, like git, URL, path, or workspace dependencies. We should definitely skip those for now similarly to what we do in cargo, by checking if a source is defined for a dependency (and later on we could support updates for git dependencies for instance).

I can make a separate PR to handle skipping those dependencies, since I believe this is not directly related to what this PR implements, as today we already consider dependencies with different sources as public PyPI ones anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created #31270 for the 2nd point, thanks for pointing that out, I should have been implemented from the start.

}

return deps;
}

Expand Down Expand Up @@ -142,6 +150,20 @@ export class UvProcessor implements PyProjectProcessor {
}
}

function extractRegistryUrls(uvManifest: Uv): string[] {
rarkins marked this conversation as resolved.
Show resolved Hide resolved
// Extra indexes have priority over default index: https://docs.astral.sh/uv/reference/settings/#extra-index-url
const registryUrls = uvManifest['extra-index-url'] ?? [];

if (uvManifest['index-url']) {
registryUrls.push(uvManifest['index-url']);
} else if (registryUrls.length) {
// If default index URL is not overridden, we need to use default PyPI URL additionally to potential extra indexes.
registryUrls.push(PypiDatasource.defaultURL);
}

return registryUrls;
}

function generateCMD(updatedDeps: Upgrade[]): string {
const deps: string[] = [];

Expand Down
72 changes: 38 additions & 34 deletions lib/modules/manager/pep621/schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,41 @@ const DependencyRecordSchema = z
.record(z.string(), z.array(z.string()))
.optional();

const PdmSchema = z.object({
'dev-dependencies': DependencyRecordSchema,
source: z
.array(
z.object({
url: z.string(),
name: z.string(),
verify_ssl: z.boolean().optional(),
}),
)
.optional(),
});

const HatchSchema = z.object({
envs: z
.record(
z.string(),
z
.object({
dependencies: DependencyListSchema,
'extra-dependencies': DependencyListSchema,
})
.optional(),
)
.optional(),
});

const UvSchema = z.object({
'dev-dependencies': DependencyListSchema,
'index-url': z.string().optional(),
'extra-index-url': z.array(z.string()).optional(),
});

export type Uv = z.infer<typeof UvSchema>;

export const PyProjectSchema = z.object({
project: z
.object({
Expand All @@ -25,40 +60,9 @@ export const PyProjectSchema = z.object({
.optional(),
tool: z
.object({
pdm: z
.object({
'dev-dependencies': DependencyRecordSchema,
source: z
.array(
z.object({
url: z.string(),
name: z.string(),
verify_ssl: z.boolean().optional(),
}),
)
.optional(),
})
.optional(),
hatch: z
.object({
envs: z
.record(
z.string(),
z
.object({
dependencies: DependencyListSchema,
'extra-dependencies': DependencyListSchema,
})
.optional(),
)
.optional(),
})
.optional(),
uv: z
.object({
'dev-dependencies': DependencyListSchema,
})
.optional(),
pdm: PdmSchema.optional(),
hatch: HatchSchema.optional(),
uv: UvSchema.optional(),
})
.optional(),
});
Expand Down