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

feat(manager/cargo): support git dependencies #32235

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions lib/modules/manager/cargo/__fixtures__/Cargo.1.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ pcap-sys = { version = "=0.1", path = "pcap-sys" }
pnet = { version = "0.21.0", optional = true, default-features = false}
git_dep_with_version = { version = "0.1.0", git = "https://github.com/foo/bar" }
git_dep = { git = "https://github.com/foo/bar" }
git_dep_with_tag = { git = "https://github.com/foo/bar", tag = "1.10.3" }
git_dep_with_rev = { git = "https://github.com/foo/bar", rev = "abc1234" }
git_dep_with_branch = { git = "https://github.com/foo/bar", branch = "next" }
same_version_1__ = "0.0.0"
same_version_1_ = "0.0.0"
same_version_1 = "0.0.0"
Expand Down
50 changes: 45 additions & 5 deletions lib/modules/manager/cargo/__snapshots__/extract.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -282,23 +282,63 @@ exports[`modules/manager/cargo/extract extractPackageFile() extracts multiple de
},
},
{
"currentValue": "0.1.0",
"datasource": "crate",
"currentValue": undefined,
"datasource": "git-refs",
"depName": "git_dep_with_version",
"depType": "dependencies",
"managerData": {
"nestedVersion": true,
},
"skipReason": "git-dependency",
"packageName": "https://github.com/foo/bar",
"skipReason": "unspecified-version",
},
{
"currentValue": "",
"datasource": "crate",
"currentValue": undefined,
"datasource": "git-refs",
"depName": "git_dep",
"depType": "dependencies",
"managerData": {
"nestedVersion": false,
},
"packageName": "https://github.com/foo/bar",
"skipReason": "unspecified-version",
},
{
"currentValue": "1.10.3",
"datasource": "github-tags",
"depName": "git_dep_with_tag",
"depType": "dependencies",
"managerData": {
"nestedVersion": false,
},
"packageName": "foo/bar",
"registryUrls": [
"https://github.com",
],
"skipReason": undefined,
},
{
"currentDigest": "abc1234",
"currentValue": "",
"datasource": "git-refs",
"depName": "git_dep_with_rev",
"depType": "dependencies",
"managerData": {
"nestedVersion": false,
},
"packageName": "https://github.com/foo/bar",
"replaceString": "abc1234",
"skipReason": undefined,
},
{
"currentValue": "next",
"datasource": "git-refs",
"depName": "git_dep_with_branch",
"depType": "dependencies",
"managerData": {
"nestedVersion": false,
},
"packageName": "https://github.com/foo/bar",
"skipReason": "git-dependency",
},
{
Expand Down
18 changes: 18 additions & 0 deletions lib/modules/manager/cargo/artifacts.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { RepoGlobalConfig } from '../../../config/types';
import * as docker from '../../../util/exec/docker';
import { ExecError } from '../../../util/exec/exec-error';
import * as _hostRules from '../../../util/host-rules';
import { CrateDatasource } from '../../datasource/crate';
import type { UpdateArtifactsConfig } from '../types';
import * as cargo from '.';

Expand Down Expand Up @@ -49,6 +50,7 @@ describe('modules/manager/cargo/artifacts', () => {
const updatedDeps = [
{
depName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down Expand Up @@ -83,6 +85,7 @@ describe('modules/manager/cargo/artifacts', () => {
const updatedDeps = [
{
depName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand All @@ -106,6 +109,7 @@ describe('modules/manager/cargo/artifacts', () => {
const updatedDeps = [
{
depName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand All @@ -132,6 +136,7 @@ describe('modules/manager/cargo/artifacts', () => {
packageName: 'dep1',
lockedVersion: '1.0.0',
newVersion: '1.0.1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down Expand Up @@ -181,6 +186,7 @@ describe('modules/manager/cargo/artifacts', () => {
packageName: 'dep1',
lockedVersion: '1.0.0',
newVersion: '1.0.1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down Expand Up @@ -290,18 +296,21 @@ describe('modules/manager/cargo/artifacts', () => {
packageName: 'dep1',
lockedVersion: '1.0.0',
newVersion: '1.0.1',
datasource: CrateDatasource.id,
},
{
depName: 'dep2',
packageName: 'dep2',
lockedVersion: '1.0.0',
newVersion: '1.0.2',
datasource: CrateDatasource.id,
},
{
depName: 'dep3',
packageName: 'dep3',
lockedVersion: '1.0.0',
newVersion: '1.0.3',
datasource: CrateDatasource.id,
},
];

Expand Down Expand Up @@ -359,6 +368,7 @@ describe('modules/manager/cargo/artifacts', () => {
{
depName: 'renamedDep1',
packageName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down Expand Up @@ -388,6 +398,7 @@ describe('modules/manager/cargo/artifacts', () => {
const updatedDeps = [
{
depName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down Expand Up @@ -428,6 +439,7 @@ describe('modules/manager/cargo/artifacts', () => {
const updatedDeps = [
{
depName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down Expand Up @@ -493,6 +505,7 @@ describe('modules/manager/cargo/artifacts', () => {
const updatedDeps = [
{
depName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down Expand Up @@ -598,6 +611,7 @@ describe('modules/manager/cargo/artifacts', () => {
const updatedDeps = [
{
depName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down Expand Up @@ -674,6 +688,7 @@ describe('modules/manager/cargo/artifacts', () => {
const updatedDeps = [
{
depName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down Expand Up @@ -733,6 +748,7 @@ describe('modules/manager/cargo/artifacts', () => {
const updatedDeps = [
{
depName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down Expand Up @@ -791,6 +807,7 @@ describe('modules/manager/cargo/artifacts', () => {
const updatedDeps = [
{
depName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down Expand Up @@ -842,6 +859,7 @@ describe('modules/manager/cargo/artifacts', () => {
const updatedDeps = [
{
depName: 'dep1',
datasource: CrateDatasource.id,
},
];
expect(
Expand Down
27 changes: 19 additions & 8 deletions lib/modules/manager/cargo/artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
} from '../../../util/fs';
import { getGitEnvironmentVariables } from '../../../util/git/auth';
import { regEx } from '../../../util/regex';
import { CrateDatasource } from '../../datasource/crate';
import type { UpdateArtifact, UpdateArtifactsResult, Upgrade } from '../types';
import { extractLockFileContentVersions } from './locked-version';

Expand Down Expand Up @@ -130,14 +131,24 @@ async function updateArtifactsImpl(
config.constraints?.rust,
);
} else {
const missingDep = updatedDeps.find((dep) => !dep.lockedVersion);
if (missingDep) {
// If there is a dependency without a locked version then log a warning
// and perform a regular workspace lockfile update.
logger.warn(
{ dependency: missingDep.depName },
'Missing locked version for dependency',
);
const nonCrateDep = updatedDeps.find(
(dep) => dep.datasource !== CrateDatasource.id,
);
const crateDepWithoutLockedVersion = updatedDeps.find(
(dep) => !dep.lockedVersion && dep.datasource === CrateDatasource.id,
);
// Non-crate dependencies (like git ones) do not have locked versions.
// For crate dependencies, not having a locked version is not expected.
// In both situations, perform a regular workspace lockfile update.
if (nonCrateDep || crateDepWithoutLockedVersion) {
Copy link
Contributor Author

@mkniewallner mkniewallner Nov 2, 2024

Choose a reason for hiding this comment

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

Not fully sure of the logic here, but without this change, we would display a warning that appears in the dependency dashboard when updating git dependencies (warning was added in #25983).

On the overall logic, technically cargo update --precise supports git dependencies, but I think the logic is different that the one for crates.

For instance on https://github.com/mkniewallner/renovate-rust-git-dependencies/blob/2eb824eb730a37b108f1d4eec951c027a031905a/Cargo.toml, when running cargo update without arguments, serde and transitive dependencies will get updated, but git dependencies will be left untouched:

 [[package]]
 name = "serde"
-version = "1.0.213"
+version = "1.0.214"
 
 [[package]]
 name = "serde_derive"
-version = "1.0.213"
+version = "1.0.214"
 
 [[package]]
 name = "syn"
-version = "2.0.85"
+version = "2.0.86"

And trying cargo update ruff_python_parser --precise 0.7.0 will lead to those changes in the lock file:

 [[package]]
 name = "ruff_python_parser"
 version = "0.0.0"
-source = "git+https://github.com/astral-sh/ruff?tag=0.6.1#499c0bd875c3f53c65f542a217b4d9a0962191c3"
+source = "git+https://github.com/astral-sh/ruff?tag=0.6.1#5e6de4e0c69660e8ca8608d1ac965216197756ce"

where the commit do resolve to 0.7.0, but while keeping 0.6.1 tag reference in the source + Cargo.toml, which feels wrong?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you split this back to Discussion - either existing or new - assuming that a design decision needs making? It's hard to follow threads in PR reviews

if (crateDepWithoutLockedVersion) {
// Only warn when a crate dependency has no locked version, as this is
// not an expected situation.
logger.warn(
{ dependency: crateDepWithoutLockedVersion.depName },
'Missing locked version for dependency',
);
}
await cargoUpdate(
packageFileName,
false,
Expand Down
2 changes: 1 addition & 1 deletion lib/modules/manager/cargo/extract.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe('modules/manager/cargo/extract', () => {
it('extracts multiple dependencies simple', async () => {
const res = await extractPackageFile(cargo1toml, 'Cargo.toml', config);
expect(res?.deps).toMatchSnapshot();
expect(res?.deps).toHaveLength(15);
expect(res?.deps).toHaveLength(18);
});

it('extracts multiple dependencies advanced', async () => {
Expand Down
12 changes: 11 additions & 1 deletion lib/modules/manager/cargo/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import type { Category } from '../../../constants';
import { CrateDatasource } from '../../datasource/crate';
import { GitRefsDatasource } from '../../datasource/git-refs';
import { GitTagsDatasource } from '../../datasource/git-tags';
import { GithubTagsDatasource } from '../../datasource/github-tags';
import { GitlabTagsDatasource } from '../../datasource/gitlab-tags';
import * as cargoVersioning from '../../versioning/cargo';
import { updateArtifacts } from './artifacts';
import { extractPackageFile } from './extract';
Expand All @@ -20,4 +24,10 @@ export const defaultConfig = {
versioning: cargoVersioning.id,
};

export const supportedDatasources = [CrateDatasource.id];
export const supportedDatasources = [
CrateDatasource.id,
GithubTagsDatasource.id,
GitlabTagsDatasource.id,
GitRefsDatasource.id,
GitTagsDatasource.id,
];
29 changes: 20 additions & 9 deletions lib/modules/manager/cargo/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { SkipReason } from '../../../types';
import { Toml, withDepType } from '../../../util/schema-utils';
import { CrateDatasource } from '../../datasource/crate';
import type { PackageDependency } from '../types';
import { applyGitSource } from '../util';
import type { CargoManagerData } from './types';

const CargoDep = z.union([
Expand All @@ -12,6 +13,12 @@ const CargoDep = z.union([
path: z.string().optional(),
/** Git URL for the dependency */
git: z.string().optional(),
/** Git tag for the dependency */
tag: z.string().optional(),
/** Git revision for the dependency */
rev: z.string().optional(),
/** Git branch for the dependency */
branch: z.string().optional(),
/** Semver version */
version: z.string().optional(),
/** Name of a registry whose URL is configured in `.cargo/config.toml` or `.cargo/config` */
Expand All @@ -25,6 +32,9 @@ const CargoDep = z.union([
({
path,
git,
tag,
rev,
branch,
version,
registry,
package: pkg,
Expand All @@ -39,15 +49,6 @@ const CargoDep = z.union([
nestedVersion = true;
} else {
currentValue = '';
skipReason = 'invalid-dependency-specification';
}

if (path) {
skipReason = 'path-dependency';
} else if (git) {
skipReason = 'git-dependency';
} else if (workspace) {
skipReason = 'inherited-dependency';
}

const dep: PackageDependency<CargoManagerData> = {
Expand All @@ -56,6 +57,16 @@ const CargoDep = z.union([
datasource: CrateDatasource.id,
};

if (path) {
skipReason = 'path-dependency';
} else if (workspace) {
skipReason = 'inherited-dependency';
} else if (git) {
applyGitSource(dep, git, rev, tag, branch);
} else if (!version) {
skipReason = 'invalid-dependency-specification';
}

if (skipReason) {
dep.skipReason = skipReason;
}
Expand Down
Loading