Skip to content

Preserve the version metadata in wit merge#2078

Merged
alexcrichton merged 2 commits intobytecodealliance:mainfrom
jsturtevant:merge-version-preservation
Mar 3, 2025
Merged

Preserve the version metadata in wit merge#2078
alexcrichton merged 2 commits intobytecodealliance:mainfrom
jsturtevant:merge-version-preservation

Conversation

@jsturtevant
Copy link
Contributor

@jsturtevant jsturtevant commented Feb 27, 2025

Follow up to #2076 where I found this issue.

It is expected that when merging two worlds the versions are preserved. For instance the merge should preserve the @since for the into and the from.

given shared world 1:

world shared-world-with-versions-and-include {
  import shared-version-on-from;
  @since(version = 1.0.0)
  import shared-version-on-into;
}

And shared world 2:

world shared-world-with-versions-and-include {
  @since(version = 1.0.0)
  import shared-version-on-from;
  import shared-version-on-into;
}

It is expected to see:

world shared-world-with-versions-and-include {
  @since(version = 1.0.0)
  import shared-version-on-from;
  @since(version = 1.0.0)
  import shared-version-on-into;
}

Signed-off-by: James Sturtevant <jstur@microsoft.com>
Signed-off-by: James Sturtevant <jstur@microsoft.com>
@jsturtevant jsturtevant force-pushed the merge-version-preservation branch from eea9a22 to 3c097eb Compare February 28, 2025 22:05
@jsturtevant jsturtevant marked this pull request as ready for review February 28, 2025 22:14
@jsturtevant
Copy link
Contributor Author

@alexcrichton This is ready for a review, I wasn't able to refactor as much as I initiall thought since the semantics in the various places were slightly different even though the code looked similar. If you have ideas or suggestions be happy to try again.

Comment on lines +697 to +710
for from_import in world.imports.iter() {
Resolve::update_world_imports_stability(
from_import,
&mut self.worlds[world_id].imports,
&interface_map,
)?;
}
for from_export in world.exports.iter() {
Resolve::update_world_imports_stability(
from_export,
&mut self.worlds[world_id].exports,
&interface_map,
)?;
}
Copy link
Member

Choose a reason for hiding this comment

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

How come this was necessary? I would have expected that by the time we merge things include has already been fully expanded and such.

Is the issue though that imports/exports have different stabilities depending on whether they came from an include or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The include is fully expanded at this point but the stability information wasn't copied over. I did look at coping the version info over when it was merging, but it looked like when the merging was done the id's were copied over and the stability updates where happening at a later point here.

Is the issue though that imports/exports have different stabilities depending on whether they came from an include or not?

even in worlds without includes the version wasn't copied over. For instance the since was missing on the from in the shared-world-with-versions. When I reversed from and into in the test it did the opposite.

world shared-world-with-versions {
  @since(version = 1.0.0)  // this was missing
  import shared-version-on-from;

Copy link
Member

Choose a reason for hiding this comment

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

Aha I understand now, thanks for expanding!

Comment on lines +697 to +710
for from_import in world.imports.iter() {
Resolve::update_world_imports_stability(
from_import,
&mut self.worlds[world_id].imports,
&interface_map,
)?;
}
for from_export in world.exports.iter() {
Resolve::update_world_imports_stability(
from_export,
&mut self.worlds[world_id].exports,
&interface_map,
)?;
}
Copy link
Member

Choose a reason for hiding this comment

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

Aha I understand now, thanks for expanding!

@alexcrichton alexcrichton added this pull request to the merge queue Mar 3, 2025
Merged via the queue into bytecodealliance:main with commit cbd6ace Mar 3, 2025
31 checks passed
duffney pushed a commit to duffney/wasm-tools that referenced this pull request Mar 4, 2025
* Preserve the version metadata in wit

Signed-off-by: James Sturtevant <jstur@microsoft.com>

* Add test case for exports

Signed-off-by: James Sturtevant <jstur@microsoft.com>

---------

Signed-off-by: James Sturtevant <jstur@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants