andir / npins

Nix dependency pinning. Very similar to Niv but has a few features that I personally wanted.
European Union Public License 1.2
164 stars 12 forks source link

Modify pins #84

Open piegamesde opened 1 month ago

piegamesde commented 1 month ago

Currently the way to modify pins is to simply re-add them with the new configuration. I think it would be good to have some shortcuts for the most common use cases:

My first idea was a generic modify command (similar to Niv), however which CLI flags are be valid would change based on the kind of the pin being modified. But also having a modify-github etc. for each kind of pin is a bit tedious.

So instead, maybe have one modify command per action or class of actions? As long as changing the source itself is not supported, all git-likes can be handled together with the same options. This would leave channels and pypi as separate though.

andir commented 1 month ago

I've thought about it a few times... So far I've just manually edited the JSON as that seemed the least troublesome to me. niv solves this by having a bit of "typelevel magic" (probably not the wrong Haskell term 😅) to modify arbitrary attributes on a pin (IIRC).

We should also consider only allowing users to change what they can already provide while adding a pin. Allowing a user to change e.g. the hash sum might have a valid use case, but those should be bugs in npins. If you need to mock with "generated" fields, something else is missing. You can always resort to JSON editing if you need that.

So the "source" and version bounds sound like a good candidate for these changes.

If we added some new trait for each kind of Pin that allowed some introspection-like behaviour for fields we can likely solve this without a ton of effort.

My rough idea would be:

pub trait Editable {
    /* const */ fn get_fields(&self) -> Vec<(FieldName, FieldType)>;
    fn modify(&mut self, field: FieldName, value: FieldValue);
}

pub enum FieldType {
    String,
    Integer,
    Url,
    Optional(FieldType),
    // ....
}

pub enum FieldValue {
    String(String),
    Integer(i64),
    Url(url::Url),
    Optional(Option<FieldValue>),
    // ...
}

impl FieldValue { 
    fn type(&self) -> FieldType {
       use Self::*;
       match self {
         String(_) => FieldType::String,
          // ...
       }
    }
}

impl Editable for GitHubRelease {
   /* const */ fn get_fields(&self) -> Vec<(FieldName, FieldType)> {
     vec![("owner".into(), FieldType::String), ("repo".into(), FieldType::String), ("version-bound".into(), FieldType::Optionall(FieldType::String))] 
  }
     fn modify(&mut self, field: FieldName, value: FieldValue) -> Error<()> {
       use FieldType::*;
        match (field, value.type()) {
            ("owner", String(s)) => self.owner = s.into(),
            ("owner", _) => return Err("invalid value for field `owner`".into()),
        }
     }
}

We can likely auto-generate most of the code if we add a list of editable fields to our already load-bearing macro.

piegamesde commented 1 month ago

I'm honestly not a big fan of Niv's approach of letting you modify individual fields. Because often times, fields depend on each other and have invariants. But also they already depend on the pin type and stuff. Abstracting over that means that the CLI cannot guide the user as to which fields can be meaningfully changed in which way.