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.

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

Expand aliases #454

Closed
chevdor opened this issue Jul 28, 2023 · 16 comments · Fixed by #458
Closed

Expand aliases #454

chevdor opened this issue Jul 28, 2023 · 16 comments · Fixed by #458
Labels
s: Daemon This issue touches pueue daemon s: Pueue-lib This issue touches the pueue-lib library t: Feature A new feature that needs implementation

Comments

@chevdor
Copy link

chevdor commented Jul 28, 2023

A detailed description of the feature you would like to see added.

Currently, the following will fail:

alias foo=date
pueue add foo

It fails because in the pueue context, user aliases are not loaded, so foo is unknown.
It would be useful to have an option (set as default ?) that loads user's env such as currently set aliases.
Adding shopt -s expand_aliases may help but I did not test.

Explain your usecase of the requested feature

pueue is very useful for a user to queue up a bunch of tasks. It is quick natural for users to already have created aliases for the individual tasks. For instance, a user may already have an alias to run freshlam && scan or brew update && brew upgrade.

Alternatives

No response

Additional context

No response

@Nukesor
Copy link
Owner

Nukesor commented Jul 28, 2023

@chevdor
Copy link
Author

chevdor commented Jul 28, 2023

uh ok, this link is a gold mine :)

@chevdor
Copy link
Author

chevdor commented Jul 28, 2023

That also provides options to #451

@chevdor
Copy link
Author

chevdor commented Jul 28, 2023

While convenient, it is not very convenient to have to refening all aliases just for pueue though.

@Nukesor
Copy link
Owner

Nukesor commented Aug 4, 2023

This was already a feature in the old pueue implementation and it was a constant pain point, since there's no good way to implement aliases.
These are the current constraints:

  1. All commands are executed by the system shell /bin/sh.
  2. The shell is a non-interactive shell, since we cannot just parse any .bashrc files. These often contain logic such as plugins that require an interactive shell and break in a non-interactive environment.
    -> This would break the shell for a majority of users.
    • An alternative would be to expect a separate .bashrc that only contains aliases, which is basically the same as the pueue_aliases.yml right now.
  3. We cannot use the user's shell, since shells use different commands/approaches to execute things and we heavily rely on the posix shell standard.

As I said earlier, this was implemented in the first version of pueue, but users entered broken states they didn't understand and that simply couldn't be fixed by any upstream changes, since these were shell specific problems.

I'm still very hesitant to allow any custom shell specific logic (or even custom shells) to be executed, unless there's a very good way of handling this.

A few fun questions from the top of my head:

  • If the daemon is on a remote machine or a different user, and has a different .rc file than the client, how should that be handled? The user most likely expects the local .rc file to be used for executing programs.
  • How to handle interactive logic in a non-interactive shell?
  • How to handle non-standard .*rc file locations

@chevdor
Copy link
Author

chevdor commented Aug 7, 2023

I understand, thanks for the explanations.
How about parsing ~/.bash_aliases and/or ~/zsh_aliases. Those are standard and could easily be parsed with a regex.

To answer your questions:

If the daemon is on a remote machine or a different user, and has a different .rc file than the client, how should that be handled? The user most likely expects the local .rc file to be used for executing programs.

Pick one of the \..*?_aliases files for the user

How to handle interactive logic in a non-interactive shell?

We don't, just fetch the aliases

How to handle non-standard .*rc file locations

We don't, just fetching aliases, like you do today, just without the need for an extra other file

@Nukesor
Copy link
Owner

Nukesor commented Aug 18, 2023

How about parsing ~/.bash_aliases and/or ~/zsh_aliases. Those are standard and could easily be parsed with a regex.

This will quickly become a problem, since there's no standard for ~/.bash_aliases. It's just a vague community practice on how one could name a file that contains aliases, but

  1. There's no rule that it has to only contain aliases. It's basically just a bash script. This makes parsing very error prone.
  2. There's no rule that it must be named that way -> We would have to make it configurable.

Then there's the other thing that people use different shells than bash and zsh, which would need to be supported as well. These shells might have a different alias syntax which would then need a different parser.

In general, I don't feel too comfortable to support specific shells or external tooling.

@Nukesor
Copy link
Owner

Nukesor commented Aug 18, 2023

The only proper way to fix this (as I currently see), is to go back and revisit allowing users to setup their own shell commands. That way, users can specify the exact way their commands should be executed, with their favorite shell flavour.

This will probably come with some other problems though. A few of Pueue's functionalities would probably need to be disabled if a custom user shell is detected, such as shellescape, which only support posix shell syntax.

@Nukesor
Copy link
Owner

Nukesor commented Aug 18, 2023

I added a configurable shell command. Could you check the MR out and see if you can use this?

@Nukesor
Copy link
Owner

Nukesor commented Aug 20, 2023

Turns out, there's apparently no way to pre-load any kind of rc file for bash or zsh in non-interactive mode.
And aliases are only saved after the current prompt has completed, when sourcing a script.

@Nukesor
Copy link
Owner

Nukesor commented Aug 21, 2023

I did some more research and managed to get this working for bash.

  shell_command: [
    "bash",
    "-c",
    "shopt -s expand_aliases && {{ pueue_command_string }}",
  ]
  env_vars:
    BASH_ENV: "/path/to/your/alias/file"

I had to add another configuration field env_vars, which allows users to inject/overwrite arbitrary environment variables into processes.

Though, I wasn't able to get this running for zsh.
The following should work in theory, but for some reason, it doesn't:

  shell_command: [
    "zsh",
    "-c",
    "source /path/to/your/alias_file && setopt aliases && {{ pueue_command_string }}",
  ]

@Nukesor Nukesor added t: Feature A new feature that needs implementation s: Pueue-lib This issue touches the pueue-lib library s: Daemon This issue touches pueue daemon labels Aug 21, 2023
@chevdor
Copy link
Author

chevdor commented Aug 22, 2023

Nice! That's promising.

@Nukesor
Copy link
Owner

Nukesor commented Aug 23, 2023

@chevdor Could you go ahead and test the branch?
I added a small guide on how to test this over there #458

If you're using some other shells, It would also be awesome if you could try to get it running!

@chevdor
Copy link
Author

chevdor commented Aug 23, 2023

I can test zsh/bash. Having a look now.

@chevdor
Copy link
Author

chevdor commented Aug 23, 2023

So it's been a bit of a fight :)

context for the commands below:

  • alias q=pueue
  • alias qd='pueued -d'
  • my .zsh_aliases file defines some aliases including p=podman

I realized that changing the config in pueue.yml required to restart the daemon. That totally makes sense but I missed that at first so my changes were not "visible".

zsh won't work unless a magic eval is issued.

I could get it to work using:

daemon:
  [...]
  shell_command: [
    "zsh",
    "-c",
    ". $HOME/.zsh_aliases; setopt aliases; eval {{ pueue_command_string }}",
  ]
  env_vars:
    # BASH_ENV: "irrelevant"

then calling:

q shutdown; qd; q clean; q add -- p --version; sleep 2; q

@Nukesor
Copy link
Owner

Nukesor commented Sep 29, 2023

The custom shell is merged, so I'll close this for now :)

@Nukesor Nukesor closed this as completed Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: Daemon This issue touches pueue daemon s: Pueue-lib This issue touches the pueue-lib library t: Feature A new feature that needs implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants