Skip to content
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.

Sign up for GitHub

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

Jump to bottom

WIP: Allow for custom registries urls #1434

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

WIP: Allow for custom registries urls #1434

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
95fdf40
Allow for custom registries urls
THenry14 Jul 9, 2024
b506092
Move url to config, allow setting using env variable
THenry14 Jul 18, 2024
e848c99
Use registry by default
THenry14 Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scarb/src/core/registry/index/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub struct IndexConfig {
}

impl IndexConfig {
pub const WELL_KNOWN_PATH: &'static str = "config.json";
pub const WELL_KNOWN_PATH: &'static str = "api/v1/index/config.json";
}

#[derive(Copy, Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)]
Expand Down
12 changes: 11 additions & 1 deletion scarb/src/core/registry/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use anyhow::Result;
use async_trait::async_trait;
use once_cell::sync::Lazy;
use std::env;

use crate::core::{ManifestDependency, Package, PackageId, Summary};

Expand All @@ -11,7 +13,15 @@ pub mod patch_map;
pub mod patcher;
pub mod source_map;

pub const DEFAULT_REGISTRY_INDEX: &str = "https://there-is-no-default-registry-yet.com";
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())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok cool! so based on this, and after discussing things with @maciektr , let's plan the work accordingly:

Sounds good?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good

});

pub fn default_registry_index() -> &'static str {
*DEFAULT_REGISTRY_INDEX
}

#[async_trait(?Send)]
pub trait Registry {
Expand Down
4 changes: 2 additions & 2 deletions scarb/src/core/source/id.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer};
use smol_str::SmolStr;
use url::Url;

use crate::core::registry::DEFAULT_REGISTRY_INDEX;
use crate::core::registry::default_registry_index;
use crate::core::source::Source;
use crate::core::Config;
use crate::internal::fsx::PathBufUtf8Ext;
Expand Down Expand Up @@ -237,7 +237,7 @@ impl SourceId {

pub fn default_registry() -> Self {
static CACHE: Lazy<SourceId> = Lazy::new(|| {
let url = Url::parse(DEFAULT_REGISTRY_INDEX).unwrap();
let url = Url::parse(default_registry_index()).unwrap();
SourceId::new(url, SourceKind::Registry).unwrap()
});
*CACHE
Expand Down
Loading