Conversation
coolreader18
left a comment
coolreader18
left a comment
There was a problem hiding this comment.
LGTM. An idea for allowing specifying the stdlib dir, you could maybe allow a DirFromEnv CompilationSourceKind?
derive/src/compile_bytecode.rs
Outdated
|
|
||
| fn compile_dir( | ||
| &self, | ||
| path: &PathBuf, |
There was a problem hiding this comment.
| path: &PathBuf, | |
| path: &Path, |
There was a problem hiding this comment.
Why is this better?
There was a problem hiding this comment.
PathBuf vs Path is akin to String vs str - an owned, allocated buffer vs a slice. Like you should (almost) always prefer &str over &String, so should you for &Path over &PathBuf.
| use ::rustpython_vm::__exports::bincode; | ||
| bincode::deserialize::<::rustpython_vm::bytecode::CodeObject>(#bytes) | ||
| .expect("Deserializing CodeObject failed") | ||
| hashmap! { |
There was a problem hiding this comment.
You should probably add hashmap to __exports and use it here.
There was a problem hiding this comment.
What is the purpose of the __exports? This is my first time using procedural macro
There was a problem hiding this comment.
I think I understand what __exports do. Added hashmap
|
Btw, I opened this issue in pyckitup: pickitup247/pyckitup#4 they might be interested in this. |
We should probably add the ability to freeze any directory.
I can get the directory from an environment variable but I hoped for something easier. |
|
Ah yeah, I think that's the best way to go for configuration, as Rust doesn't have anything like that built in. We've done similar configuration in env variables before, e.g. |
|
I sorta glazed over this when I reviewed earlier, but I don't think that it should always return a |
I am not sure about that... I prefer that |
3 participants
As discussed in #1142.
Adding the current
Libfolder increase the executable by 5MB. I would like to add this to the demo so it will support python stdlib.I wanted to allow specifying the location of the stdlib dir with a compilation flag but could not find out how to do that. @coolreader18 any suggestions?