ETVYFS4YF4KPUGJ2AFIVMHKL2ZNEPQYHGOW45HNR5ZJPAQIPG6GQC
OTA33L6RLEFKRJBFMCN5FOX5LYFHGM6FK2PIL3YKVWFJYLIFAMJAC
The first change implements the change to have pijul
run hooks directly rather than through bash
. It also includes the relevant changes to ./.pijul/config
to support the change as I outlined in my initial post.
The second change adds an optional label
field to hooks. If present, the ‘label’ will be used in the error message about the hook failing. If not present, the command
of the hook is used instead.
So given a config for hooks of:
record = [ { label = "format", command = "cargo", args = [ "fmt" ] } ]
If the hook fails the error message is: Hook "format" exited with code ExitStatus(ExitStatus(1))
JU6IAXZJRGANRUVLLDRWS3XGSTP47UIHRKOPQHPXSHTAZD3WHIYQC
Realized that the command
field should not default to an empty string when not present. The error message is still not great, but it is better than the OS error from trying to pass an empty string to std::process::Command::new()
.
ZOLH5DSPWMU2PBBUGUPDET6GAU7IOLZED2RBBDJPPCYXZPDYKJGQC
I made squashed version (ZOLH5DSPWMU2PBBUGUPDET6GAU7IOLZED2RBBDJPPCYXZPDYKJGQC) of the previous changes.
Hi! Thanks for your change. This looks good, but would need to be Windows-specific, as I believe that many Unix users don’t always realise what is bash syntax (~
, *
, among others) and what is actually sent to commands. That said, I also understand that having a different syntax for Unix and Windows might lead to highly confusing answers on StackOverflow.
Do you know what happens if one uses environment variables or wildcards (*
) in hooks on Windows?
Neither of the Windows shells (cmd and powershell) support file globbing. They both have different syntax for environment variables from each other and from Unix shells. ‘cmd’ does environment variables like %PATH%
. ‘powershell’ does them like $ENV:PATH
. The latter is why I was having trouble with getting Pijul’s logging to work. In powershell, $RUST_LOG="pijul=debug"' sets a script variable called 'RUST_LOG', but it needs to be
$ENV:RUST_LOG=“pijul=debug”` to set the environment variable other programs (like Pijul) can access.
This change was a first step as I wanted to talk about it before going further. The next step is allowing shells to be configured in the global configuration file, and the hooks can use those shells. Also it could be set up so users can configure a default shell to use in the global and or per repository configuration file(s).
Have shells be explicit helps make it clear that the proper way to write hooks will depend on the shell being used.
As to people being unclear about what Unix shells are responsible for, in the manual there should be a special note that for hooks that use file globbing should be set to use a shell such as Bash. Such a note would be good in both the ‘Getting Started’ (suggesting Unix users set up their usual shell and make it the default) and in the documentation for hooks.
HCH4BFPMUCK2KLMI5RRRBAM6TWXZ5HUVXF7KOXWFOZEX5745VZGAC
Hi! Actually, the first example in the documentation for Command
shows how to do this!
What do you think of the patch I just pushed?
No rush, there are plenty of things to fix before we move to beta.
It works.
Here is a thought, it would be possible to allow to have an optional entry in to config to set a shell to use on a per repository basis.
Something like:
[shell]
exec = "/bin/zsh"
flag = "-c"
and on Windows to use PowerShell instead of cmd
[shell]
exec = "powershell.exe" //or `exec = "C:\\Program Files\\PowerShell\\7\\pwsh.exe"` to use PowerShell 7 instead of the PowerShell 5 that comes with Windows 10
flag = "-Command"
If not present, then pijul
would default to what you have implemented in the change.
Good point: we might want to use a different shell sometimes.
That may complicate the configuration file though, what would you think about a slightly more general solution where we can add an option to bypass the shell entirely for some hooks, like:
[hooks]
record = [ "cargo fmt", { command = "/bin/zsh", args = [ "-c", "cargo fmt" ] } ]
VL7ZYKHBPKLNY5SA5QBW56SJ7LBBCKCGV5UAYLVF75KY6PPBOD4AC
Adding the ability to configure an alternative shell to use in a repository is something that can be waited on to see if there is enough demand for it.
Being able to bypass the shell would likely cover most cases where being able to set an alternative would be wanted.
Alright, I’m closing then, let’s see if someone ever needs this.
Currently, when
pijul
runs hooks set up in.pijul/config
, it runsbash
to execute the hook commands.It is only possible to rely on
bash
on Windows if it is installed along sidepijul
and that instance ofbash
is used.While it might be possible to use one of
cmd
orpowershell
on Windows (or have a configured shell in the global~/.config/pijul/config.toml
), I have one other possible direction to suggest.It would also be possible to call the commands directly without a shell. One down side is that hooks that run shell script files rather than executables would be more verbose to configure (though it would be the same as running a different shell that the one
pijul
uses). Also with the way hooks are currently configured it would not be possible to set up arguments for them.So the configuration for hooks would need to be changed if
pijul
is to run commands directly. Example of current hook configuration:Example rewritten to be suitable for
pijul
to run hook commands directly:Which is more verbose to write. But it makes it easy to make the code for running hooks portable. It would also allow for some future additions such as being able to configure environment variables for a hook command.
It would also be possible to not change the config if
pijul
could parse the command string, but that is quite a bit more work unless there is a good crate for doing that kind of thing.