WIP: Allow for custom registries urls #1434
Conversation
scarb/src/core/registry/mod.rs
Outdated
| static DEFAULT_REGISTRY_INDEX: Lazy<&'static str> = Lazy::new(|| { | ||
| let registry_url = env::var("SCARB_REGISTRY_URL") | ||
| .unwrap_or_else(|_| "https://there-is-no-default-registry-yet.com".into()); | ||
| Box::leak(registry_url.into_boxed_str()) |
There was a problem hiding this comment.
I would be happy to hear any thoughts about this, as I'm not sure about its implications. Box::leak is used to convert the String to a &'static str by leaking the heap-allocated memory. This should be safe in this context because Lazy ensures the value is only initialized once and lives for the entire program duration, but still this may not be THE optimal way to achieve this.
There was a problem hiding this comment.
You can technically keep Lazy<String> there and it'd be more collect.
But I think this is not the best approach for implementing this. I'd rather put this into Config, so that this could eventually be stored in some kind of a file. After all, someone might want to use a custom proxy for example and they'd like to write the URL once to some file and leave it.
There was a problem hiding this comment.
Sure, could make more sense, thanks for pointers.
btw is this something that, in your opinion, could be settable from the manifest file? Just wondering how I should approach this
There was a problem hiding this comment.
Definitely not from the manifest because this is something user-specific, not project
Cargo has this: https://doc.rust-lang.org/cargo/reference/config.html
There was a problem hiding this comment.
Ok cool! so based on this, and after discussing things with @maciektr , let's plan the work accordingly:
- I will first redo this PR, to read the url from a flag and hold it in the
Configstruct - I will then implement reading this from said file, at some later date, in a different PR under this issue Implement Config deserialization to config file #1441
Sounds good?
THenry14
changed the title
WIP: please do not review yet
The PR aims to allow scarb to use a custom registry, by defining an env variable