-
Notifications
You must be signed in to change notification settings - Fork 5
reading cache directory during runtime without dir crate #5
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Please make sure to appease CI and also sign the commit if possible
src/lib.rs
Outdated
| #[cfg(feature = "disk-persistence")] | ||
| pub use setup::get_storage_dir; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this have to be re-exported / public api? If not then let's avoid that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the re-exporting
src/setup.rs
Outdated
| #[cfg(feature = "disk-persistence")] | ||
| fn get_storage_dir() -> Option<PathBuf> { | ||
| dirs::cache_dir().map(|mut path| { | ||
| pub fn get_storage_dir() -> Option<PathBuf> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have two functions: get_storage_path and get_storage_dir. Since we are making changes here, is it possible to just make it one function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged the two in the new update
tests/arkworks/setup.rs
Outdated
| if let Some(cache_dir) = dirs::cache_dir() { | ||
| if let Some(cache_dir) = get_storage_dir() { | ||
| let cache_file = cache_dir | ||
| .join("dory") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .join("dory") |
This might be adding an extra /dory now that I am looking at it, which would silently fail to clean up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I missed that. Fixed it now
e8df2f0 to
a098f89
Compare
Signed-off-by: Amirhossein Khajehpour <khajepour.amirhossein@gmail.com>
Signed-off-by: Amirhossein Khajehpour <khajepour.amirhossein@gmail.com>
a098f89 to
4d3e0ab
Compare
markosg04
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! Will merge this into the next release.
d28dec9 to
e578dac
Compare
dir crate can not be compiled by the verifier in jolt which leads to proof recursion failure.
This PR removes dir dependency and manually determines caching directory.