[Feature request]: Add per-package config options about pacdef HOT 17 OPEN

@ripytide Feel free to open WIP PRs for this if that's okay with you. That helps me keeping track of what's gonna happen when multiple people work on different things for this project.

from pacdef.

@InnocentZero This comment here has the syntax for the group files: #79 (comment) and this here has some implementation guidelines. #79 (comment)

As for refactoring other backend things, let's approach that when concrete questions come up.

Sounds good?

And feel free to ask questions anytime, I'm glad to help :)

from pacdef.

That's an interesting idea. I think this is actually two separate features.

pacman has optional dependencies that it may not install unless explicitly stated

That would be something that the AUR helper needs to do, not pacdef, since we do not care about dependencies in that manner. So the question is "Does paru have an option like paru -S --includeoptionaldeps pacman"?

Or you could just check add the optional dependencies you want as pacdef packages, maybe even put them in their own separate group with pacman.

So far I'm hesistant regarding this.

Some packages, like pacdef itself, when installed from cargo, need feature flags to get some features.

That makes sense overall. I can think of a couple of potential issues. Let's say we go from ini

[rust]
pacdef

to this toml

[rust]
pacdef = { features = "arch" }

How do we keep track of the feature flags that were used to compile before? We will need to recompile it when the flags change.

Also, toml does not seem to support single values like this

[rust]
pacdef

but requires a key-value-pair. (That highlight is from GH's syntax parser, indicating a syntax error)

There's some stuff that will need good thinking for a good design.

from pacdef.

That would be something that the AUR helper needs to do, not pacdef, since we do not care about dependencies in that manner. So the question is "Does paru have an option like paru -S --includeoptionaldeps pacman"?

Now that you say it, it makes sense for it to be a paru option rather than a pacdef option. Though I can still think of a usecase where we mention some optional dependencies but that is just doing nix at that point.

How do we keep track of the feature flags that were used to compile before? We will need to recompile it when the flags change.

I think this case can be handled by the user (not saying it should, but I can't come up with a better solution atm). Also, seems like a feature for something like topgrade and not pacdef ig.

Also, toml does not seem to support single values like this

[rust]
pacdef

but requires a key-value-pair. (That highlight is from GH's syntax parser, indicating a syntax error)

Oh ok. I didn't know that. That is news to me.

There's some stuff that will need good thinking for a good design.

I can think of a couple of "solutions":

• We have a new major release deprecating ini and replacing it with toml/something else, but that will be quite jarring to end users.
• We have parsers for both but that complicates the entire codebase.

So I don't think either of the options are very feasible atm. We need to come up with something better.

from pacdef.

One way to accomplish this in toml might be to use arrays:

arch = [
    "hyprland",
    "pacdef",
    { package = "neovim", features = ["wl-clipboard"] },
]

It is slightly more annoying to type with all the quotation marks but it's easy to parse and extend the syntax. It's reminds me of the Cargo.toml format for dependencies.

from pacdef.

An array would work, but still requires a lot of boilerplate which I personally don't like.

I'd rather extend the custom parser we have right now.

Maybe (just spitballing here)

[rust]
pacdef, features = arch  # a comment 
topgrade
...

That is as simple as it gets I believe. Then we would need to specify valid key-value-pairs or flags (maybe someone wants to not strip a binary for whatever reason) per backend. That should be manageable.

What do you guys think?

from pacdef.

I agree with @steven-omaha. The issue at hand is not to fit TOML format for our purposes but to try and not break existing group files. Arrays sound like a good idea but once again, break backwards compatibility. If there is a method to not make users pull their hair while using TOML I'm all in for it though. Rust closely integrates with TOML anyways.

from pacdef.

I have no preference on TOML vs the current format, either seems fine to me. And backwards compatibility is certainly important so that favors keeping the current format I guess.

Some packages, like pacdef itself, when installed from cargo, need feature flags to get some features.

On this point, is there a reason why pacdef makes the arch and debian packages optional features? We could sidestep this problem entirely if all packages (such as pacdef itself) didn't have different build options, especially since such build options don't play nice with binary installs with the exponential growth of feature combinations.

All of my packages are such "simple" packages and so I am dubious of adding this complexity without significant benefit.

from pacdef.

On this point, is there a reason why pacdef makes the arch and debian packages optional features? We could sidestep this problem entirely if all packages (such as pacdef itself) didn't have different build options, especially since such build options don't play nice with binary installs with the exponential growth of feature combinations.

I think you're misunderstanding me. The point of this is to have per-backend feature flags. So a "feature = arch" kind of thing will be specific to cargo. Similarly, an "optional-dep = wl-clipboard" will only be specific to arch. I also believe this can greatly simplify the Rustup backend, which atm behaves very differently from other backends.

from pacdef.

On this point, is there a reason why pacdef makes the arch and debian packages optional features?

Whenever possible I tried to query other backends by their corresponding libraries (alpm and libapt-pkg). When there's a major change, things break in a predictable manner (failing to run or failing to compile), which seemed preferable to a package manager changing its output, which we then parse wrongly and produce bugs that are hard to track down or cause other weird issues.

from pacdef.

Okay, then let's do it this way. @InnocentZero I'd recommend the following approach.

• Package needs to hold some arbitrary key-value-pairs, so it should be extended by a pub attributes: Option<BTreeMap>.
• I think it makes sense to split the parsing of Strings into Packages into a separate module or a separate struct PackageParser that neatly contains the logic of generating a package. Then only Package with its public fields gets passed around the program.
• Every backend that needs to act on these attributes will need to have some code added. As you suggested, it makes sense to do this for rust at the moment, to enable compile-time features.

Does that make sense?

from pacdef.

That could work, but would be very stringly typed and so we wouldn't get many compiler checks for options after parsing.

Also Option<BTreeMap> is redundant as you can just use BTreeMap and use an empty map for no configs.

An alternative would be to express in the backend trait an associated Package type so every backend could define its own strongly typed package options which would then be handed in via the various trait methods.

The disadvantage of this method is it might make looping through every backend more difficult since package types would be specialized to individual backends but I think that shouldn't be too hard to handle with a few assistive meta-types.

from pacdef.

hat could work, but would be very stringly typed and so we wouldn't get many compiler checks for options after parsing.

Indeed.

Also Option is redundant as you can just use BTreeMap and use an empty map for no configs.

Agreed.

An alternative would be to express in the backend trait an associated Package type so every backend could define its own strongly typed package options which would then be handed in via the various trait methods.

That sounds reasonable.

The disadvantage of this method is it might make looping through every backend more difficult since package types would be specialized to individual backends but I think that shouldn't be too hard to handle with a few assistive meta-types.

So when passing packages to and from a backend, we may would need to convert from a BackendPackage to Package or the other way around. As long as there is a clear interface I think that shouldn't be too hard, unless I'm missing something.

Could you elaborate more about meta types in this context?

from pacdef.

I'm thinking mainly of types such as

pub struct ToDoPerBackend(Vec<(AnyBackend, Packages)>);

Which might have to change to something like:

pub struct TodoPerBackend(BTreeMap<AnyBackend, AnyPackages>);
// or
pub struct TodoPerBackend(Vec<BackendWithPackages>)
pub enum BackendWithPackages {
    Arch(Arch, Packages<ArchPackage>),
    Debian(Debian, Packages<DebianPackage>),
}

from pacdef.

Or actually maybe even:

pub struct TodoPerBackend {
    arch: Packages<ArchPackage>,
    debian: Packages<DebianPackage>,
    //etc..
}

from pacdef.

Or actually maybe even:

pub struct TodoPerBackend {
    arch: Packages<ArchPackage>,
    debian: Packages<DebianPackage>,
    //etc..
}

This sounds good to me. But did we decide on the group file syntax yet? I'll open a WIP PR once we settle on it.

from pacdef.

Just FYI, I'm halfway-through some more refactors at the moment which may end up overhauling the Backend trait. You can see where I'm at on my rework2 branch. Hopefully I'll have finished it by tomorrow.

from pacdef.