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

[Call-by-name] Investigate why so many intrinsic migrations are missing (e.g. Digest code) #20745

Open
sureshjoshi opened this issue Apr 2, 2024 · 2 comments
Assignees
Labels
enhancement external-plugins Issues related to plugins outside this repository

Comments

@sureshjoshi
Copy link
Member

This will end up being a larger ticket, as more of the codebase is converted to call-by-name syntax.

So instead of trying to bulk fill it in the existing PR and leave that rotting, better to cut that off more cleanly and move investigations to a separate ticket

@sureshjoshi sureshjoshi self-assigned this Apr 2, 2024
@huonw huonw added the external-plugins Issues related to plugins outside this repository label Apr 7, 2024
@sureshjoshi
Copy link
Member Author

@stuhood @benjyw

Trying to investigate why we have no migration data for the intrinsics.

As an example, there is no mention of RemovePrefix in the migration plan, even though those Gets are definitely there.

pants migrate-call-by-name --json | grep RemovePrefix

The associated intrinsic code appears to be present:

"remove_prefix_request_to_digest",

Ditto for the associated QueryRules:

def rules():

So, I guess: Why is there no migration plan for intrinsics (it's more than just the filesystem, for example await Get(BuiltPackage, PackageFieldSet, field_set) also isn't present).

@sureshjoshi
Copy link
Member Author

This feels like the missing piece of the puzzle.

impl Intrinsic {
pub fn new(name: &str, product: TypeId, input: TypeId) -> Self {
// TODO: Python rule code that calls an intrinsic by name will need to be
// able to import that name, so we'll need to create stubs representing the
// intrinsics, in pants.engine.intrinsics.py.
Self {
id: RuleId::from_string(format!("pants.engine.intrinsics:{}", name)),
product,
inputs: vec![DependencyKey::new(input)],
}
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement external-plugins Issues related to plugins outside this repository
Projects
None yet
Development

No branches or pull requests

2 participants