Conversation
huitseeker
added
the
no changelog
Update MAST package structure for project assembly
processor: expose advice, memory and transcript
feat: implement project assembly
Keep runtime dependencies from preassembled packages in dependency resolution. Programs that declare a kernel now require the matching kernel package when they are rebuilt. The resolver prefers the stored kernel, falls back to a matching embedded kernel, and rejects bad or conflicting embedded kernel metadata.
huitseeker
force-pushed
the
backport/main-post-v0.22.0-to-next
branch
from
88af767 to
b40e5e9
Compare
bitwalker
left a comment
bitwalker
left a comment
There was a problem hiding this comment.
I think it probably would be best to address the packaging/project assembly issues as a separate PR(s) (see #2944 for the tracking issue created for this purpose).
I think raising an error if an executable package doesn't have an embedded kernel is probably fine. I'm less sure about the other change to the dependency graph - or at least, I think a different approach might ultimately be better for resolving the issue around source packages failing to rebuild due to missing dependencies in the registry (see my other comments for details).
Could we perhaps remove that latter change, but keep the rejection of executables with no embedded kernel, and then address the other issue separately as a follow-on? Definitely feel free to add another sub-issue to #2944 if none of those existing issues covers what you were aiming to fix here.
| .map_err(|error| Report::msg(error.to_string()))?; | ||
| } | ||
|
|
||
| for node in graph.nodes.values() { |
There was a problem hiding this comment.
We only embed the kernel into executable packages - and executable packages can't be dependencies. In all other cases, we treat the dependency on a kernel like any other dependency - we can select any kernel version so long as it satisfies the version requirements of all the dependents.
|
|
||
| let record = PackageRecord::new(kernel_version, kernel_requirements); | ||
| registry | ||
| .insert_record(kernel_package.name.clone(), record) |
There was a problem hiding this comment.
This is inserting the embedded kernel into the registry, but I don't think we should be doing that - the kernel by definition would have needed to be a dependency in the graph, so there should already be a node corresponding to what was declared in the package manifest. If there isn't, something else is wrong.
See #2946 for how I propose we address the issue that I believe you are aiming to address here (though correct me if I'm wrong on that point).
This reverts commit b40e5e9.
Preassembled packages still need their runtime dependencies during resolution. Programs that need a kernel now fail when that kernel cannot be resolved, instead of silently falling back to embedded metadata during dependency selection.
huitseeker
force-pushed
the
backport/main-post-v0.22.0-to-next
branch
from
6e485a1 to
6811ce7
Compare
|
OK, I scaled this back @bitwalker :
|
2 participants
Merges main back into next.
This also keeps the main behavior for preassembled packages with kernel runtime dependencies. Package resolution keeps the kernel requirement, prefers the stored kernel when present, falls back to a matching embedded kernel when needed, and rejects bad or conflicting embedded kernel metadata. See 88af767 individually.