Working with cargo #47
Merged
+277
−57
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently defaulting to `stable_mir_json` which is this package that runs `main.rs`. But if flag `--bin cargo_stable_mir_json` is added it runs the `src/bin/cargo_stable_mir_json.rs`
jberthold
reviewed
Feb 13, 2025
There was a problem hiding this comment.
Comments are a few nit-picks on details...
The approach as a whole seems reasonable for distribution - maybe there would be a more canonical way to place things inside $HOME/.cargo but if this works we can go with this for now.
Can you add a usage example here, and a description in the README.md?
|
Might be good to give users the option of a custom install path. |
Co-authored-by: Jost Berthold <jost.berthold@gmail.com>
…p_artefacts_from_profile`
dkcumming
commented
Feb 16, 2025
jberthold
approved these changes
Feb 16, 2025
src/bin/cargo_stable_mir_json.rs
Outdated
| fn setup(repo_dir: PathBuf, maybe_user_provided_dir: Option<PathBuf>) -> Result<()> { | ||
| let smir_json_dir = smir_json_dir(maybe_user_provided_dir)?; | ||
| println!("Creating {} directory", smir_json_dir.display()); | ||
| std::fs::create_dir(&smir_json_dir)?; // This errors is the directory already exists |
There was a problem hiding this comment.
This errors is the directory already exists ... not sure that's desirable. We can leave it that way for now, though...
Also, when I suggested the user-defined location I thought this would be the final destination, not that a directory would be created there.
Co-authored-by: Jost Berthold <jost.berthold@gmail.com>
|
Added the other issue here #48 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This didn't end up looking anything like what I had imagined. But I was having a lot of trouble getting this working at all and I may have had my environment set up in a way that was self defeating as I feel I suddenly started having things work when trying the same thing. Not sure.
But right now this is creating a
.stable_mir_jsondirectory at a given directory or~/.stable_mir_jsondirectory (linux only right now) and that is populated with the binary forstable_mir_json, a file containing theLD_LIBRARY_PATHthat was used to create the binary, and scripts to set the library path so that RUSTC can be overwritten when calling cargo.To test this out:
cargo buildandcargo build --releasecargo run --bin cargo_stable_mir_json -- $PWD [optional-dir-path]from the root of the repo (this creates.stable_mir_json)RUSTC=~/.stable_mir_json/debug.sh cargo build(or the file that.stable_mir_jsonwas built in). Or for releaseRUSTC=~/.stable_mir_json/release.sh cargo buildFixes #39