-
Notifications
You must be signed in to change notification settings - Fork 18
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
Implement freezing and thawing of pins #78
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -386,6 +386,13 @@ pub struct ImportFlakeOpts { | |||||
pub name: Option<String>, | ||||||
} | ||||||
|
||||||
#[derive(Debug, StructOpt)] | ||||||
pub struct FreezeOpts { | ||||||
/// Names of the pin(s) | ||||||
#[structopt(required = true)] | ||||||
pub names: Vec<String>, | ||||||
} | ||||||
|
||||||
#[derive(Debug, StructOpt)] | ||||||
pub enum Command { | ||||||
/// Intializes the npins directory. Running this multiple times will restore/upgrade the | ||||||
|
@@ -414,6 +421,12 @@ pub enum Command { | |||||
|
||||||
/// Try to import entries from flake.lock | ||||||
ImportFlake(ImportFlakeOpts), | ||||||
|
||||||
/// Freeze a pin entry | ||||||
Freeze(FreezeOpts), | ||||||
|
||||||
/// Thaw a pin entry | ||||||
Thaw(FreezeOpts), | ||||||
} | ||||||
|
||||||
use structopt::clap::AppSettings; | ||||||
|
@@ -589,6 +602,11 @@ impl Opts { | |||||
|
||||||
if opts.names.is_empty() { | ||||||
for (name, pin) in pins.pins.iter_mut() { | ||||||
if pin.is_frozen() { | ||||||
log::info!("Skipping pin {} as it is frozen.", name); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit for consistency:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I was planning on doing this for the entire code base. We've a few cases where that isn't the case. |
||||||
continue; | ||||||
} | ||||||
|
||||||
log::info!("Updating '{}' …", name); | ||||||
self.update_one(pin, strategy, true).await?; | ||||||
} | ||||||
|
@@ -597,6 +615,11 @@ impl Opts { | |||||
match pins.pins.get_mut(name) { | ||||||
None => return Err(anyhow::anyhow!("Could not find a pin for '{}'.", name)), | ||||||
Some(pin) => { | ||||||
if pin.is_frozen() { | ||||||
log::info!("Skipping {} as it is frozen.", name); | ||||||
continue; | ||||||
} | ||||||
|
||||||
log::info!("Updating '{}' …", name); | ||||||
self.update_one(pin, strategy, true).await?; | ||||||
}, | ||||||
|
@@ -663,6 +686,43 @@ impl Opts { | |||||
Ok(()) | ||||||
} | ||||||
|
||||||
async fn freeze(&self, o: &FreezeOpts) -> Result<()> { | ||||||
let mut pins = self.read_pins()?; | ||||||
|
||||||
for name in o.names.iter() { | ||||||
let pin = match pins.pins.get_mut(name) { | ||||||
None => return Err(anyhow::anyhow!("Couldn't find the pin {} to freeze.", name)), | ||||||
Some(pin) => pin, | ||||||
}; | ||||||
|
||||||
pin.freeze(); | ||||||
log::info!("Froze pin {}", name); | ||||||
} | ||||||
|
||||||
self.write_pins(&pins)?; | ||||||
|
||||||
Ok(()) | ||||||
} | ||||||
|
||||||
async fn thaw(&self, o: &FreezeOpts) -> Result<()> { | ||||||
let mut pins = self.read_pins()?; | ||||||
|
||||||
for name in o.names.iter() { | ||||||
let pin = match pins.pins.get_mut(name) { | ||||||
None => return Err(anyhow::anyhow!("Couldn't find the pin {} to thaw.", name)), | ||||||
Some(pin) => pin, | ||||||
}; | ||||||
|
||||||
pin.thaw(); | ||||||
|
||||||
log::info!("Thawed pin {}", name); | ||||||
} | ||||||
|
||||||
self.write_pins(&pins)?; | ||||||
|
||||||
Ok(()) | ||||||
} | ||||||
|
||||||
async fn import_niv(&self, o: &ImportOpts) -> Result<()> { | ||||||
let mut pins = self.read_pins()?; | ||||||
|
||||||
|
@@ -829,6 +889,8 @@ impl Opts { | |||||
Command::Remove(r) => self.remove(r)?, | ||||||
Command::ImportNiv(o) => self.import_niv(o).await?, | ||||||
Command::ImportFlake(o) => self.import_flake(o).await?, | ||||||
Command::Freeze(o) => self.freeze(o).await?, | ||||||
Command::Thaw(o) => self.thaw(o).await?, | ||||||
}; | ||||||
|
||||||
Ok(()) | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,14 +123,16 @@ macro_rules! mkPin { | |
version: Option<<$input_name as Updatable>::Version>, | ||
#[serde(flatten)] | ||
hashes: Option<<$input_name as Updatable>::Hashes>, | ||
#[serde(default)] | ||
frozen: Frozen, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why doesn't our CI/rustfmt complain on this line? (Mixed usage of tabs and spaces) |
||
} | ||
),* | ||
} | ||
|
||
impl Pin { | ||
/* Constructors */ | ||
$(fn $lower_name(input: $input_name, version: Option<<$input_name as Updatable>::Version>) -> Self { | ||
Self::$name { input, version, hashes: None } | ||
Self::$name { input, version, hashes: None, frozen: Frozen::default() } | ||
})* | ||
|
||
/* If an error is returned, `self` remains unchanged */ | ||
|
@@ -149,7 +151,7 @@ macro_rules! mkPin { | |
*/ | ||
async fn fetch(&mut self) -> Result<Vec<diff::DiffEntry>> { | ||
Ok(match self { | ||
$(Self::$name { input, version, hashes } => { | ||
$(Self::$name { input, version, hashes, .. } => { | ||
let version = version.as_ref() | ||
.ok_or_else(|| anyhow::format_err!("No version information available, call `update` first or manually set one"))?; | ||
/* Use very explicit syntax to force the correct types and get good compile errors */ | ||
|
@@ -177,16 +179,39 @@ macro_rules! mkPin { | |
$(Self::$name { ..} => $human_name ),* | ||
} | ||
} | ||
|
||
|
||
/// Thaw a pin | ||
pub fn thaw(&mut self) { | ||
match self { | ||
$(Self::$name { ref mut frozen, .. } => frozen.thaw()),* | ||
} | ||
} | ||
|
||
/// Freeze a pin | ||
pub fn freeze(&mut self) { | ||
match self { | ||
$(Self::$name { ref mut frozen, .. } => frozen.freeze()),* | ||
} | ||
} | ||
|
||
/// Is frozen | ||
pub fn is_frozen(&self) -> bool { | ||
match self { | ||
$(Self::$name { frozen, .. } => frozen.is_frozen()),* | ||
} | ||
} | ||
} | ||
|
||
impl std::fmt::Display for Pin { | ||
fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { | ||
match self { | ||
$(Self::$name { input, version, hashes } => { | ||
$(Self::$name { input, version, hashes, frozen } => { | ||
/* Concat all properties and then print them */ | ||
let properties = input.properties().into_iter() | ||
.chain(version.iter().flat_map(Diff::properties)) | ||
.chain(hashes.iter().flat_map(Diff::properties)); | ||
.chain(hashes.iter().flat_map(Diff::properties)) | ||
.chain(frozen.properties()); | ||
for (key, value) in properties { | ||
writeln!(fmt, " {}: {}", key, value)?; | ||
} | ||
|
@@ -259,6 +284,40 @@ impl diff::Diff for GenericVersion { | |
} | ||
} | ||
|
||
/// The Frozen field in a Pin | ||
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] | ||
pub struct Frozen(bool); | ||
|
||
impl Frozen { | ||
fn new(value: bool) -> Self { | ||
Frozen(value) | ||
} | ||
|
||
fn freeze(&mut self) { | ||
self.0 = true; | ||
} | ||
|
||
fn thaw(&mut self) { | ||
self.0 = false; | ||
} | ||
|
||
fn is_frozen(&self) -> bool { | ||
self.0 | ||
} | ||
} | ||
|
||
impl diff::Diff for Frozen { | ||
fn properties(&self) -> Vec<(String, String)> { | ||
vec![("frozen".into(), self.0.to_string())] | ||
} | ||
} | ||
|
||
impl std::default::Default for Frozen { | ||
fn default() -> Self { | ||
Frozen(false) | ||
} | ||
} | ||
|
||
/// An URL and its hash | ||
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] | ||
pub struct GenericUrlHashes { | ||
|
@@ -287,3 +346,38 @@ async fn main() -> Result<()> { | |
opts.run().await?; | ||
Ok(()) | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
||
#[cfg(test)] | ||
fn test_frozen() { | ||
assert!(!Frozen::default().is_frozen()); | ||
assert!(!{ | ||
let mut x = Frozen::default(); | ||
x.thaw(); | ||
x | ||
} | ||
.is_frozen()); | ||
assert!({ | ||
let mut x = Frozen::default(); | ||
x.freeze(); | ||
x | ||
} | ||
.is_frozen()); | ||
assert!(Frozen::new(true).is_frozen()); | ||
assert!({ | ||
let mut x = Frozen::new(true); | ||
x.freeze(); | ||
x | ||
} | ||
.is_frozen()); | ||
assert!(!{ | ||
let mut x = Frozen::new(true); | ||
x.thaw(); | ||
x | ||
} | ||
.is_frozen()); | ||
} | ||
} |
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.
I get that logically
freeze
<->thaw
makes sense but from a UX perspective I would prefer to have a more explicit indication that those two commands belong together.Hence I'd prefer
freeze
andunfreeze
- WDYT @andir ?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.
I'd also prefer
unfreeze
, with an optional alias tothaw
for those who find that more intuitive