Skip to content
Merged
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
47 changes: 32 additions & 15 deletions bloom/generators/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,22 +189,39 @@ def _normalize_cargo_manifest(self, sub_dir=None):
if not os.path.isfile(manifest):
return

print('Normalizing Cargo.toml')
pkg = None
if isinstance(self.packages, dict):
pkg = self.packages.get(sub_dir)
if pkg is None:
pkg = self.packages.get('.') or self.packages.get('')
if pkg is None and len(self.packages) == 1:
pkg = list(self.packages.values())[0]

if pkg is None or pkg.get_build_type() != 'cargo':
return

with open(manifest, 'rb') as f:
data = tomllib.load(f)

for spec in (
spec for category in (
'dependencies',
'dev-dependencies',
'build-dependencies',
) for spec in data.get('dependencies', {}).values()
modified = False
for category in (
'dependencies',
'dev-dependencies',
'build-dependencies',
):
if not isinstance(spec, dict):
continue

if spec.pop('path', None):
spec.setdefault('version', '*')

with open(manifest, 'wb') as f:
tomli_w.dump(data, f)
for spec in data.get(category, {}).values():
if not isinstance(spec, dict):
continue

path_val = spec.get('path')
if path_val:
norm_path = os.path.normpath(path_val)
if not os.path.isabs(norm_path) and norm_path.split(os.sep)[0] == '..':
spec.pop('path', None)
spec.setdefault('version', '*')
modified = True

if modified:
print('Normalizing Cargo.toml')
with open(manifest, 'wb') as f:
tomli_w.dump(data, f)
135 changes: 135 additions & 0 deletions test/unit_tests/test_generators/test_release.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import os
import shutil
import tempfile
import tomli_w

try:
import tomllib
except ImportError:
import tomli as tomllib

from bloom.generators.release import ReleaseGenerator


class MockPackage:
def __init__(self, build_type):
self._build_type = build_type

def get_build_type(self):
return self._build_type


def test_normalize_cargo_manifest():
# Create a temporary directory for the test
temp_dir = tempfile.mkdtemp()
try:
manifest_path = os.path.join(temp_dir, 'Cargo.toml')

# Define the original manifest content
original_manifest = {
'dependencies': {
'absolute_dep': {'path': '/abs/path/to/dep'},
'sibling_nested_dep': {'path': '../parent/dep'},
'sibling_dep': {'path': '../sibling'},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In my mental model, ../sibling it's a sibling of my current crate, correct, but I don't see why ../parent/dep it's a parent. Looks like it's a child of another sibling, a nephew?

Maybe I'm getting something wrong

@cottsay cottsay Sep 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is maybe just a mismatch in nomenclature between what I asked Gemini to do and how I write the commit description.

All of these paths are from the perspective of a package at it's root directory. We archive the packages from that directory, so anything above it (starting with ../) is removed by Bloom's package-level processing and will NOT be present when the package is built on the buildfarm.

Maybe we just call these sibling_dep and nested_sibling_dep. Doesn't really matter, either way they should be removed (and possibly converted to a version = '*' constraint).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we just call these sibling_dep and nested_sibling_dep

That change sounds good to me then, that renaming would makes sense from the description and matches the test behavior

'subdir_dep': {'path': 'subdir/dep'},
'dot_subdir_dep': {'path': './subdir/dep'},
'nested_subdir_dep': {'path': 'subdir/sub2/../../subdir/sub3'},
'not_dict_dep': '1.0.0',
},
'dev-dependencies': {
'dev_sibling_dep': {'path': '../dev-dep'},
'dev_subdir_dep': {'path': 'dev-subdir'},
},
'build-dependencies': {
'build_sibling_dep': {'path': '../build-dep'},
'build_subdir_dep': {'path': 'build-subdir'},
}
}

# Case 1: Package of type 'cmake' (should NOT be modified or written to)
with open(manifest_path, 'wb') as f:
tomli_w.dump(original_manifest, f)

# Set modification time to the past
past_time = os.path.getmtime(manifest_path) - 100
os.utime(manifest_path, (past_time, past_time))

generator = ReleaseGenerator()
generator.packages = {temp_dir: MockPackage('cmake')}

generator._normalize_cargo_manifest(temp_dir)

# Assert no change to content and no write (modification time remains the same)
with open(manifest_path, 'rb') as f:
data = tomllib.load(f)
assert data == original_manifest
assert os.path.getmtime(manifest_path) == past_time

# Case 2: Package of type 'cargo' with no relative dependencies (should NOT be modified or written to)
manifest_no_sibling_deps = {
'dependencies': {
'absolute_dep': {'path': '/abs/path/to/dep'},
'subdir_dep': {'path': 'subdir/dep'},
}
}
with open(manifest_path, 'wb') as f:
tomli_w.dump(manifest_no_sibling_deps, f)

os.utime(manifest_path, (past_time, past_time))

generator.packages = {temp_dir: MockPackage('cargo')}
generator._normalize_cargo_manifest(temp_dir)

# Assert no change to content and no write (modification time remains the same)
with open(manifest_path, 'rb') as f:
data = tomllib.load(f)
assert data == manifest_no_sibling_deps
assert os.path.getmtime(manifest_path) == past_time

# Case 3: Package of type 'cargo' with parent relative dependencies (should normalize only parent relative dependencies)
with open(manifest_path, 'wb') as f:
tomli_w.dump(original_manifest, f)

generator._normalize_cargo_manifest(temp_dir)

with open(manifest_path, 'rb') as f:
data = tomllib.load(f)

# Let's verify each dependency after normalization
deps = data['dependencies']

# Absolute path - preserved!
assert deps['absolute_dep']['path'] == '/abs/path/to/dep'
assert 'version' not in deps['absolute_dep']

# Parent referencing path - dropped and version set to *
assert 'path' not in deps['sibling_nested_dep']
assert deps['sibling_nested_dep']['version'] == '*'

assert 'path' not in deps['sibling_dep']
assert deps['sibling_dep']['version'] == '*'

# Subdirectory path - preserved!
assert deps['subdir_dep']['path'] == 'subdir/dep'
assert 'version' not in deps['subdir_dep']

assert deps['dot_subdir_dep']['path'] == './subdir/dep'
assert 'version' not in deps['dot_subdir_dep']

assert deps['nested_subdir_dep']['path'] == 'subdir/sub2/../../subdir/sub3'
assert 'version' not in deps['nested_subdir_dep']

# dev-dependencies
dev_deps = data['dev-dependencies']
assert 'path' not in dev_deps['dev_sibling_dep']
assert dev_deps['dev_sibling_dep']['version'] == '*'
assert dev_deps['dev_subdir_dep']['path'] == 'dev-subdir'

# build-dependencies
build_deps = data['build-dependencies']
assert 'path' not in build_deps['build_sibling_dep']
assert build_deps['build_sibling_dep']['version'] == '*'
assert build_deps['build_subdir_dep']['path'] == 'build-subdir'

finally:
shutil.rmtree(temp_dir)
Loading