Make source links relative to OUT_DIR #9580
Conversation
When generating code, `cranelift-isle` records the path to the source files. These paths were sometimes absolute, meaning it kept a trace of the system it was run on. These changes here add helper functions in `Files` to try and strip the value of `$OUT_DIR` (if set) from the file names.
|
@cfallin here are some changes related to our discussion here.
About this ☝️ I'm happy to remove it/submit it separately/make it relative. @fitzgen you mentioned that, in the past, files were not printed with absolute paths. Is it worth investing some time to ensure this doesn't happen again in the future? Note: I originally changed the code to strip |
| Err(_) => file_name, | ||
| Ok(root) => file_name.strip_prefix(&root).unwrap_or(file_name).into(), | ||
| } | ||
| } |
There was a problem hiding this comment.
I wonder if the build script that invokes cranelift-isle can read this instead. Cranelift-isle can technically be used to build arbitrary code. Files::from_paths could get a set of paths to make make input paths relative to, or alternatively a set of paths against which to resolve relative paths and then have the build script only pass the relative paths for individual isle files instead.
There was a problem hiding this comment.
Sorry, not too familiar with the code/cranelift. Are you suggesting that we should make the paths relative here instead?
There was a problem hiding this comment.
Yes. There is already a make_isle_source_path_relative function which gets used, but that only affects isle files that are part of the source rather than those that get generated by the build script itself. You did still have to separately pass a (set of) prefix(s) to join with the relative path to get a path that can be passed to fs::read, but this prefix can be ignored when printing the source locations in the generated file.
There was a problem hiding this comment.
only affects isle files that are part of the source rather than those that get generated by the build script itself.
Unrelated to this thread, but that explains why I had to use OUT_DIR instead of CARGO_MANIFEST_DIR !
Yes.
That makes sense. I'll try to find some time later to update the PR.
github-actions
bot
added
cranelift
Subscribe to Label Action
This issue or pull request has been labeled: "cranelift", "cranelift:meta", "isle"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
Fixes #9553
When generating code,
cranelift-islerecords the path to the source files. These paths were sometimes absolute, meaning it kept a trace of the system it was run on.These changes here add helper functions in
Filesto try and strip the value of$OUT_DIR(if set) from the file names.This also removes a log line (which included the full path of files) from
cranelift-codegen-meta.