Skip to content

Commit 3d1f1ff

Browse files
authored
track full_names config setting more precisely (#216)
Previously, if an app depended on any Python module with a `componentize-py.toml` containing `full_names = true`, that setting would cause full names to be emitted for _all_ interfaces, not just the ones covered by that module. This commit makes the effect of that setting more precise such that only the interfaces covered by that module are affected.
1 parent 911ef76 commit 3d1f1ff

4 files changed

Lines changed: 50 additions & 29 deletions

File tree

src/lib.rs

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use {
1818
path::{Path, PathBuf},
1919
str,
2020
},
21-
summary::{Locations, Summary},
21+
summary::{Locations, Naming, Summary},
2222
tar::Archive,
2323
wasm_encoder::{CustomSection, Section as _},
2424
wasmtime::{
@@ -234,13 +234,12 @@ impl BindingsGenerator<'_> {
234234
let stream_and_future_indexes = &HashMap::new();
235235
let summary = Summary::try_new(
236236
&resolve,
237-
&iter::once(world).collect(),
237+
&iter::once((world, Naming::from_full(self.full_names))).collect(),
238238
self.import_interface_names,
239239
self.export_interface_names,
240240
import_function_indexes,
241241
export_function_indexes,
242242
stream_and_future_indexes,
243-
self.full_names,
244243
)?;
245244
let world_module = self.world_module.unwrap_or(DEFAULT_WORLD_MODULE);
246245
let world_dir = self.output_dir.join(world_module.replace('.', "/"));
@@ -447,12 +446,23 @@ impl ComponentGenerator<'_> {
447446
let mut all_worlds = worlds
448447
.iter()
449448
.copied()
450-
.chain(configs.values().flat_map(|(_, v)| v.iter().copied()))
451-
.collect::<IndexSet<_>>();
449+
.map(|world| (world, Naming::from_full(self.full_names)))
450+
.chain(configs.values().flat_map(|(config, worlds)| {
451+
worlds.iter().copied().map(|world| {
452+
(
453+
world,
454+
Naming::from_full(config.config.full_names || self.full_names),
455+
)
456+
})
457+
}))
458+
.collect::<IndexMap<_, _>>();
452459

453460
if all_worlds.is_empty() {
454461
// No worlds specified; pick the default one, if available:
455-
all_worlds.insert(select_world(&resolve, None, &packages)?);
462+
all_worlds.insert(
463+
select_world(&resolve, None, &packages)?,
464+
Naming::from_full(self.full_names),
465+
);
456466
}
457467

458468
// Now that we've parsed all known WIT files and resolved all relevant
@@ -482,15 +492,15 @@ impl ComponentGenerator<'_> {
482492

483493
let world = unioned(
484494
&mut resolve,
485-
&all_worlds.iter().copied().collect::<Vec<_>>(),
495+
&all_worlds.keys().copied().collect::<Vec<_>>(),
486496
)?
487497
.unwrap();
488498

489499
// Determine which worlds are covered by which Python modules and, for
490500
// each module, union the ones covered by the module into a single
491501
// world.
492502

493-
let mut worlds_to_generate = all_worlds.clone();
503+
let mut worlds_to_generate = all_worlds.keys().copied().collect::<IndexSet<_>>();
494504

495505
let configs = configs
496506
.iter()
@@ -592,13 +602,6 @@ impl ComponentGenerator<'_> {
592602
&imported_function_indexes,
593603
&exported_function_indexes,
594604
&stream_and_future_indexes,
595-
// TODO: We should restrict the `full_names` setting found in a give
596-
// config file to only the world(s) covered by that config file, if
597-
// feasible.
598-
self.full_names
599-
|| configs
600-
.values()
601-
.any(|(config, ..)| config.config.full_names),
602605
)?;
603606

604607
let need_async = summary.need_async();
@@ -814,7 +817,11 @@ impl ComponentGenerator<'_> {
814817
async move {
815818
let component = &Component::new(&engine, instrumented)?;
816819
if !added_to_linker {
817-
add_wasi_and_stubs(&resolve, &all_worlds, &mut linker)?;
820+
add_wasi_and_stubs(
821+
&resolve,
822+
&all_worlds.keys().copied().collect::<IndexSet<_>>(),
823+
&mut linker,
824+
)?;
818825
}
819826

820827
let pre = InitPre::new(linker.instantiate_pre(component)?)?;

src/summary.rs

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,18 @@ const NOT_IMPLEMENTED: &str = "raise NotImplementedError";
3131

3232
const ASYNC_START_PREFIX: &str = "_async_start_";
3333

34+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
35+
pub enum Naming {
36+
Short,
37+
Full,
38+
}
39+
40+
impl Naming {
41+
pub fn from_full(full: bool) -> Self {
42+
if full { Self::Full } else { Self::Short }
43+
}
44+
}
45+
3446
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)]
3547
pub enum Direction {
3648
Import,
@@ -93,6 +105,7 @@ pub struct InterfaceInfo<'a> {
93105
package: Option<PackageName<'a>>,
94106
name: &'a str,
95107
docs: Option<&'a str>,
108+
naming: Naming,
96109
}
97110

98111
struct FunctionCode {
@@ -145,21 +158,18 @@ pub struct Summary<'a> {
145158
imported_function_indexes: &'a HashMap<(Option<&'a str>, &'a str), usize>,
146159
exported_function_indexes: &'a HashMap<(Option<&'a str>, &'a str), usize>,
147160
stream_and_future_indexes: &'a HashMap<TypeId, usize>,
148-
full_names: bool,
149161
need_async: bool,
150162
}
151163

152164
impl<'a> Summary<'a> {
153-
#[expect(clippy::too_many_arguments)]
154165
pub fn try_new(
155166
resolve: &'a Resolve,
156-
worlds: &IndexSet<WorldId>,
167+
worlds: &IndexMap<WorldId, Naming>,
157168
import_interface_names: &HashMap<&str, &str>,
158169
export_interface_names: &HashMap<&str, &str>,
159170
imported_function_indexes: &'a HashMap<(Option<&'a str>, &'a str), usize>,
160171
exported_function_indexes: &'a HashMap<(Option<&'a str>, &'a str), usize>,
161172
stream_and_future_indexes: &'a HashMap<TypeId, usize>,
162-
full_names: bool,
163173
) -> Result<Self> {
164174
let mut me = Self {
165175
resolve,
@@ -181,24 +191,25 @@ impl<'a> Summary<'a> {
181191
imported_function_indexes,
182192
exported_function_indexes,
183193
stream_and_future_indexes,
184-
full_names,
185194
need_async: false,
186195
};
187196

188197
let mut import_keys_seen = HashSet::new();
189198
let mut export_keys_seen = HashSet::new();
190-
for &world in worlds {
199+
for (&world, &naming) in worlds {
191200
me.visit_functions(
192201
&resolve.worlds[world].imports,
193202
Direction::Import,
194203
world,
195204
&mut import_keys_seen,
205+
naming,
196206
)?;
197207
me.visit_functions(
198208
&resolve.worlds[world].exports,
199209
Direction::Export,
200210
world,
201211
&mut export_keys_seen,
212+
naming,
202213
)?;
203214
}
204215

@@ -388,6 +399,7 @@ impl<'a> Summary<'a> {
388399
direction: Direction,
389400
world: WorldId,
390401
keys_seen: &mut HashSet<WorldKey>,
402+
naming: Naming,
391403
) -> Result<()> {
392404
for (key, item) in items {
393405
self.world_keys
@@ -433,6 +445,7 @@ impl<'a> Summary<'a> {
433445
package,
434446
name: item_name,
435447
docs: interface.docs.contents.as_deref(),
448+
naming,
436449
};
437450

438451
self.resource_state = Some(ResourceState { direction });
@@ -1193,7 +1206,7 @@ impl<'a> Summary<'a> {
11931206
.or_default()
11941207
.entry(info.package.map(|p| (p.namespace, p.name)))
11951208
.or_default()
1196-
.insert(info.package.and_then(|p| p.version), id)
1209+
.insert(info.package.and_then(|p| p.version), (id, info.naming))
11971210
.is_none()
11981211
);
11991212
}
@@ -1202,11 +1215,11 @@ impl<'a> Summary<'a> {
12021215
for (name, packages) in &tree {
12031216
for (package, versions) in packages {
12041217
if let Some((package_namespace, package_name)) = package {
1205-
for (version, id) in versions {
1218+
for (version, &(id, naming)) in versions {
12061219
assert!(
12071220
names
12081221
.insert(
1209-
*id,
1222+
id,
12101223
if let Some(version) = version {
12111224
if let Some(name) = interface_names.get(
12121225
format!(
@@ -1216,7 +1229,7 @@ impl<'a> Summary<'a> {
12161229
.as_str(),
12171230
) {
12181231
(*name).to_owned()
1219-
} else if versions.len() == 1 && !self.full_names {
1232+
} else if versions.len() == 1 && naming == Naming::Short {
12201233
if packages.len() == 1 {
12211234
(*name).to_owned()
12221235
} else {
@@ -1233,7 +1246,7 @@ impl<'a> Summary<'a> {
12331246
.as_str()
12341247
) {
12351248
(*name).to_owned()
1236-
} else if packages.len() == 1 && !self.full_names {
1249+
} else if packages.len() == 1 && naming == Naming::Short {
12371250
(*name).to_owned()
12381251
} else {
12391252
format!("{package_namespace}-{package_name}-{name}",)
@@ -1246,7 +1259,7 @@ impl<'a> Summary<'a> {
12461259
assert!(
12471260
names
12481261
.insert(
1249-
*versions.get(&None).unwrap(),
1262+
versions.get(&None).unwrap().0,
12501263
(*interface_names.get(*name).unwrap_or(name)).to_owned()
12511264
)
12521265
.is_none()
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
wit_directory = "wit"
22
bindings = "wit"
3+
full_names = true

src/test/python_source/app.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,6 @@ class FooInterface(foo_exports.FooInterface):
246246
def test(self, s: str) -> str:
247247
return foo_test(f"{s} FooInterface.test")
248248

249-
class BarInterface(bar_exports.BarInterface):
249+
class BarSdkBarInterface(bar_exports.BarSdkBarInterface):
250250
def test(self, s: str) -> str:
251251
return bar_test(f"{s} BarInterface.test")

0 commit comments

Comments
 (0)