Show commands based on distro family

[SoapyDev]

Is your feature request related to a problem? Please describe. Some command that are implemented are not ready to be used on every distribution

Describe the solution you'd like A way to fix this would be to detect the distribution family from the executable and display the commands corresponding to it.

There is multiple ways to approach this problem and multiple emerging benefits and issues that comes out of them.

1) Adding a list of valid distro to a command and call one bash file that cover all cases / exception This has the lowest amount of duplication, but any in the bash script might introduce bugs. It bars the way to people that might be very well versed with Debian, but not so much with Arch for example. Add maintenance in Rust.

2) Split the bash files per distro family and detect if the current distro as a file This has the benefit to require little maintenance in rust. This has the benefit to be granular and very targeted, but we get more duplication and possible unequal behaviors.

3) Adding a bash script that return if we should display the command This has the benefit to require little maintenance in rust. Very easy to put in place. It would be possible to split the files per distro family if we return the path to the script. It would be possible to keep it as one single bash file. Can be templated Can add exception for some distro, rather than blocking a whole family

There is many other ways to approach this, so I opened this issue to get feedback. My main concern is that some amazing script are coming in, but they are not meant to be used on every distribution from the get go. So they need to be or refused, or become a liability.

I think this project needs a mechanism to accept any good script, without the risk that comes with it.

@ChrisTitusTech, @JustLinuxUser , @lj3954 , what are your thought on this subject ?

[afonsofrancof]

I am pretty sure that we want to have every command work on every distro we want to support. So when adding a new command, it needs to support everything. I don't think we currently have any command that is distro specific, and I don't think @ChrisTitusTech wants to add that.

If that is the case, I think the best way would be to add a function to https://github.com/ChrisTitusTech/linutil/blob/main/src/commands/common-script.sh, which checks the distro that is running the tool. That file is sourced in every script, so it could just export a $LINUTIL_DISTRO variable that can then be used by them.

If we don't want to support the same distros on every file (which, again, I think we do), we could use the first approach of having a vector of supported distros in our rust code for each command. There is a downside to this, which is that we need to make sure that each command file handles all possible distros that are listed in the rust code, which could be out of sync by accident.

[lj3954]

I am pretty sure that we want to have every command work on every distro we want to support. So when adding a new command, it needs to support everything. I don't think we currently have any command that is distro specific, and I don't think @ChrisTitusTech wants to add that.

If that is the case, I think the best way would be to add a function to https://github.com/ChrisTitusTech/linutil/blob/main/src/commands/common-script.sh, which checks the distro that is running the tool. That file is sourced in every script, so it could just export a $LINUTIL_DISTRO variable that can then be used by them.

If we don't want to support the same distros on every file (which, again, I think we do), we could use the first approach of having a vector of supported distros in our rust code for each command. There is a downside to this, which is that we need to make sure that each command file handles all possible distros that are listed in the rust code, which could be out of sync by accident.

We already have cases where incompatibilities arise depending on the system (xrandr display control scripts). We also have more similar proposed changes, such as the theming script which isn't supposed to be used on DEs. I think it's quite a good idea to allow scripts to specify necessary preconditions, and to hide them if they're not met (possibly with an override flag). I think the talk here should center around how such a feature should be implemented.

A proposal like #140 could be relevant. Perhaps this proposed JSON data could include an array of environment variables or files with an array of values that either needs to contain the content of the variable/file or exclude it. Here's an example of what that code could look like.

#[derive(Deserialize)]
struct ScriptData {
    #[serde(default)]
    preconditions: Option<Vec<Precondition>>,
}

#[derive(Deserialize)]
struct Precondition {
    // If true, the data must be contained within the list of values.
    // Otherwise, the data must not be contained within the list of values
    matches: bool,
    data: DataType,
    values: Vec<String>,
}

#[derive(Deserialize)]
enum DataType {
    Environment(String),
    File(PathBuf),
}

Then, we could easily check this data at runtime without a significant performance hit. This would be an optional field (as is enumerated in the example), so any script which doesn't need these checks would have no need to add anything extra to the JSON data. Data that could be read are files such as /etc/os-release (for this issue's example), variables like XDG_SESSION_TYPE (for xorg/wayland only scripts) or XDG_CURRENT_DESKTOP (for scripts which only work on specific window managers or desktop environments).

[lj3954]

I've implemented this idea on top of the tablist: https://github.com/lj3954/linutil/tree/simple_additions. Input would be helpful.

[SoapyDev]

@lj3954, I like this quite a lot, but I am questioning why you went for a 1:1 ratio on the json/commands, rather than something like a single json file per tab or for the whole project with a line defining the tab?

Also, I think sometimes, defining what to exclude, might be faster computing wise and human wise than defining the list of accepted cases.

For example, lets say we have a command that is working across all Arch based distro, but one of the distro makes a change that breaks compatibility.

The best case scenario would be to exclude the given distro and not Arch as a whole by not having to define the whole list of working distros.

So adding something like an exclude field and watching for it before we match positively might be something desirable and faster in this case (that can be added after this is accepted :) ).

I will also add an idea in here to get it out of my head and once this is accepted I will open an other ticket. An experimental field (bool). By default, those are hidden, if the command line -- -e or --experimental is given, they are shown (possibly with an indicator next to the name of the command). This would also help @ChrisTitusTech to accept PRs that are interesting but not ready with minimal changes.

[lj3954]

I like this quite a lot, but I am questioning why you went for a 1:1 ratio on the json/commands, rather than something like a single json file per tab or for the whole project with a line defining the tab?

I'll look into this. Doing a JSON per tab sounds like an interesting idea. I think it could get quite cumbersome with tabs containing more entries, though, like the utilities one which contains 15 or so scripts.

defining what to exclude, might be faster

That's what the 'matching' bool is for. Setting it to true ensures the value exists, setting it to false ensures it doesn't.

An experimental field

I disagree. Large changes often include refactors which would break the existing code. My PR with tabs is a good example, it is completely incompatible with the previous structure. The point of a master branch is that changes get merged in when they're just about ready. Stable releases serve the purpose you're speaking about.

[JustLinuxUser]

I think doing such checks on a script by script level is a better idea, that will reduce code duplication. I see a possibility of having SOME distro specific options that appear in the menu based on the distro, but I don't think this is a big enough edge case to warrant a rewrite of 50% of linutil, and if such a necessity occurs, we can just print an error in the shell script. Something like "This option is specific to Debian" or something like that.

This is a solution in search of a problem, and the problem is nowhere to be found

[lj3954]

but I don't think this is a big enough edge case to warrant a rewrite of 50% of linutil

Please remember my PR is based on the tablist branch (#136), only the commits following that are related to the JSON. The changes specific to that PR are much smaller, see commits past the first. It just replaces the fixed tablist with JSON parsing and tree creation, and adds the necessary JSON files. JSON could also be (drop in) replaced with TOML if it's expected that contributors who aren't as familiar with Rust will be able to understand and write it easily.

Rewriting some of linutil isn't also the worst idea. The code has to be organized much better. You can see examples of these changes in #137, #151, #136, #120, and #145. Leaving just about everything in main and list is unreasonable, especially as the project grows.

This 'edge case' is already used. Linutil has xrandr monitor control scripts, which of course only are compatible with x11. Therefore, I've used this new JSON to ensure the x11 display server is being used, with this line in each of those scripts { "matches": true, "data": { "environment": "XDG_SESSION_TYPE" }, "values": ["x11"] }.

I do agree that the case of requiring a specific distro is extremely niche and likely would rarely be used. But the option is still given, in case the script author needs it. There are significantly more important things that many scripts will depend on, such as a specific desktop environment or display server.

[JustLinuxUser]

Apparently I am missing some important context here, I don't have much time to spend on linutil this month, so sorry for my uninformed intrusion, and yes, if you already decided to go JSON, adding one more env variable should not be to big of a change. Going JSON at all does seam really weird to me though, but then again I lack all the context and unable to provide a better solution right now due to the lack of time. I will try to catch up to the project at the end of August