Running a command in a subdir which needs file paths that are absolute or relative to the subdir

[oalders]

I've got a configuration for black which is kind of interesting. The command needs to be run from inside a ./pylib directory because of virtualenv. We've worked around this by having a wrapper script which will precious calls. That script will cd pylib before running black.

However, if we do this, precious hands off paths with are prefixed pylib/ and black cannot find the files. The next step would be to turn the relative URL into an absolute. So I end up with something like:

#!/bin/bash

FILE=$1
if [[ ${#@} > 1 ]]; then
    ARG=$1
    FILE=$2
fi

BASE=$PWD

# This script is a wrapper used by precious. It exists as there doesn't seem to
# be an easy way to make precious change to an arbitrary working dir before
# running a command.
cd pylib

pipenv --bare --quiet run \
    black $ARG "$BASE/$FILE"

If black is run as a linter, it needs a --check arg, so the wrapper (clumsily) deals with the fact that a switch could precede the filename.

I don't know if there's an elegant way to deal with this in precious. If there were options to have precious:

• cd into an arbitrary directory
• pass absolute file paths

then I think it would cover this case. FWIW I'm going to have to do something similar for mypy and pylint, so I thought it was worth raising here.

[autarch]

I've been thinking a fair bit recently about the run_mode and chdir options. I think there's a lot more ways that people might want to run a command than can be expressed currently. So far here's what I have:

1. Run the command once from the root, without passing any paths = run_mode = "root", chdir = true.
2. Run the command once from the root, passing the root dir as "." = run_mode = "root", chdir = false.
3. Run the command once from the root, passing all files as absolute paths in a single invocation = impossible with current options
4. Run the command once per directory, chdir into that directory, without passing any paths = run_mode = "dirs", chdir = true.
5. Run the command once per directory, chdir into that directory, passing the path as "." = impossible with current options.
6. Run the command once per directory, chdir into that directory, passing all files in that directory as relative paths in a single invocation = impossible with current options.
7. Run the command once per directory without chdir, passing all files as absolute paths in that directory = impossible with current options.
8. Run the command once per file, chdir into that file's directory, passing the relative path to that file = run_mode = "files", chdir = true.
9. Run the command once per file, without chdir, passing the absolute path to that file = run_mode = "files", chdir = false.

And for all of these I think there is a variant where we do this but we treat a single subdirectory as the root.

It sounds like for your particular use case you want #3 with the "subdirectory as the new root" option. Is that correct?

I've been thinking of replacing run_mode and chdir with something new. I'm not sure exactly what though. Should I reuse those config key names (supporting the existing ones via backcompat)? Add more config keys that work with those two? Add a new set of config keys to replace the existing ones (supporting the existing ones via backcompat)?

Off-hand, I think something like this might work;

invoke = { "once" | "per-dir" | "per-file" }
run-in = { "root" | "dir" }
file_args = { "none" | "absolute-dir" | "absolute-file" | "relative-dir" | "relative-file" }
with-root = "path/to/subdir"

Though I think this has some nonsensical combos, like invoke = "per-file" + "file_args = "none"`. This definitely requires some more thought!

[oalders]

This is an interesting matrix. 😄 I think for my case I'd be looking at "Run the command once per file, chdir into that file's directory passing the absolute path to that file".

invoke = "per-file"
run-in = "dir"
file-args = "absolute-file"
with-root = "pylib"

So, I think those configuration options would cover it nicely. I think they're also clearer than run_mode, which I had to think more about.

[autarch]

I realized that we only need three keys, since run-in and with-root are both just ways to change the working directory for command execution. That simplifies things and reduces the number of nonsensical combos. I documented how it would work in a branch at https://github.com/houseabsolute/precious/blob/invoke/README.md#command-invocation

If you have any feedback on this I'd appreciate it.

[oalders]

Great! A couple of thoughts:

1. wd as a name is not immediately clear to me. Even calling it something like workdir would make it easier to understand the config file without having to reference the commands at the same time

2. Having path-args of none is helpful. I didn't see that option in the current CLI and I do have a script where I'm working around the fact that a . is passed as the first arg

This looks really good. I think it would cover my use cases.

[autarch]

I have a build of the new code up at https://github.com/houseabsolute/precious/actions/runs/3358319122

Would you be up for giving it a try? You can see the new docs in the invoke branch. Also see new the new docs dir there.

[oalders]

Would you be up for giving it a try?

Yes. I will test drive it today and let you know. Thanks for this!

[oalders]

Just had a quick look and a couple of things.

1. The example I copy/pasted from the new docs is not validating:

[command.some-linter]
invoke = "per-file"
working_dir = {
    "sub_roots" = [
        "pkg1",
        "pkg2",
    ]
}
path_args = "file"

It works when changed to

[command.some-linter]
invoke = "per-file"
working_dir = { "sub_roots" = [ "pkg1","pkg2", ] }
path_args = "file"

(Not sure why).

Also, I'm not sure if the new options get me the invocation that I need. I want precious to

• chdir to the pylib folder
• Run the command once per file using an absolute filename

[commands.black]
type = "both"
lint_flags = "--check"
invoke = "per-file"
working_dir = { "sub_roots" = [ "pylib" ] }
path_args = "absolute-file"
cmd = ["pipenv", "--bare", "--quiet", "run", "black"]
ignore_stderr = [".*reformatted.*", "would reformat.*", ".* left unchanged",]
ok_exit_codes = 0
include = [ "**/*.py" ]

The error message that I get back is:

Error: the command black could not be found within PATH or Pipfile's [scripts].

That's an error that I get if I try to run the command outside of the pylib dir. If I change to invoke = "per-file" then it works for all files which are inside of pylib, but not for those outside of it.

I checked and apparently we're abusing pipenv, so this may not be an obvious use case.

[autarch]

Just had a quick look and a couple of things.

1. The example I copy/pasted from the new docs is not validating:

[command.some-linter]
invoke = "per-file"
working_dir = {
    "sub_roots" = [
        "pkg1",
        "pkg2",
    ]
}
path_args = "file"

It works when changed to

[command.some-linter]
invoke = "per-file"
working_dir = { "sub_roots" = [ "pkg1","pkg2", ] }
path_args = "file"

(Not sure why).

Oh, I think there's a better way to write this, which is:

working_dir.sub_roots = [
    "pkg1",
    "pkg2",
]

I suspect the way I first wrote it doesn't work because TOML doesn't allow all the line breaks.

Also, I'm not sure if the new options get me the invocation that I need. I want precious to

• chdir to the pylib folder
• Run the command once per file using an absolute filename

[commands.black]
type = "both"
lint_flags = "--check"
invoke = "per-file"
working_dir = { "sub_roots" = [ "pylib" ] }
path_args = "absolute-file"
cmd = ["pipenv", "--bare", "--quiet", "run", "black"]
ignore_stderr = [".*reformatted.*", "would reformat.*", ".* left unchanged",]
ok_exit_codes = 0
include = [ "**/*.py" ]

The error message that I get back is:

Error: the command black could not be found within PATH or Pipfile's [scripts].

That's an error that I get if I try to run the command outside of the pylib dir. If I change to invoke = "per-file" then it works for all files which are inside of pylib, but not for those outside of it.

I checked and apparently we're abusing pipenv, so this may not be an obvious use case.

The config you gave should do what you want. Can you run precious --debug ... and paste the output here? Hopefully that'll give us some clue what's going on. Just run it with one file. If you do it with --all or a whole directory the output will be really long.

[autarch]

Doh, I just realized that a bunch of integration tests I thought were running were actually a no-op. So I don't think this code actually works yet!

[oalders]

So I don't think this code actually works yet!

I re-ran the command with --debug and it doesn't actually chdir to a subroot. It just runs the command from the root, so I think that may be the issue.

[autarch]

Alright, now I think it might actually work. I realized I hadn't even implemented the sub-roots code at all before!

https://github.com/houseabsolute/precious/actions/runs/3405255971

[oalders]

This works for everything in my pylib folder. 🎊

The remaining issue is that I have some python scripts in folders outside of pylib which are now not being linted. The directory structure is something like:

dev/
├── bin
│   └── bar
│       └── outside.py
pylib/
|--baz
|  └── inside.py

and the config:

[commands.black]
type = "both"
lint_flags = "--check"
invoke = "per-file"
working_dir = { "sub_roots" = [ "pylib" ] }
path_args = "absolute-file"
cmd = ["pipenv", "--bare", "--quiet", "run", "black"]
ignore_stderr = [".*reformatted.*", "would reformat.*", ".* left unchanged",]
ok_exit_codes = 0
include = [ "**/*.py" ]
exclude = [
  "ansible/**/*",
  "web/**/*",
]

precious lint --command black --all will now lint everything in pylib, but it ignores dev/lib. What I need it to do is find all of the matching files, regardless of where they are in the directory structure, chdir to ./pylib and then run the linter from that directory.

I don't see a way to do this currently. I do understand that this is probably a non-standard use case, but if there were a way to say "get all the matching files and just run all of the commands from this arbitrary root", that would cover this case.

[autarch]

What I need it to do is find all of the matching files, regardless of where they are in the directory structure, chdir to ./pylib and then run the linter from that directory.

I don't see a way to do this currently. I do understand that this is probably a non-standard use case, but if there were a way to say "get all the matching files and just run all of the commands from this arbitrary root", that would cover this case.

Yeah, there's no way to do that even with the new options.

I think one way that might work is to add an option that looks like this:

working_dir.root = "pylib"

I don't think this would be too hard to add. But I do wonder how likely anyone else is to have this use case.

It seems like you could easily write a small shell wrapper for black that would just change to the correct dir, munge the filenames it was given to add ../ at the front, and then run black itself.

[oalders]

But I do wonder how likely anyone else is to have this use case.

That's what I was thinking. 🤔

It seems like you could easily write a small shell wrapper for black that would just change to the correct dir, munge the filenames it was given to add ../ at the front, and then run black itself.

Right. I've got a few variations of:

#!/bin/bash

set -eu -o pipefail

BASE=$PWD

# This script is a wrapper used by precious. It exists as there doesn't seem to
# be an easy way to make precious change to an arbitrary working dir before
# running a command.
cd pylib

if [[ ${#@} -gt 1 ]]; then
    PIPENV_VERBOSITY=-1 pipenv --bare --quiet run \
        black "$1" "$BASE/$2"
else
    PIPENV_VERBOSITY=-1 pipenv --bare --quiet run \
        black "$BASE/$1"
fi

It was just a bit of a pain to do it bash, so I figured I'd at least mention it here. It's not a deal-breaker by any stretch.

[autarch]

It does occur to me that this exposes something about the new config options that is less than ideal. Basically, these are the things we want to control.

1. How many times should it be invoked. This is now invoke and can be per file, per dir, or just once.
2. What the working directory be when it's run. This is either always the root, each directory in turn, or 1+ sub-roots.
3. What paths to pass to the command.
4. How do we find the files that are relevant to the command.

So 1-3 correspond to invoke, working_dir, and path_args, but for #4 there's no explicit config. Instead, it's an implicit setting based on working_dir, and you can't set this separately from the working directory.

So maybe it'd make sense to add a fourth config item, called something like find_files_in which can be used to set where to look for files. But this is really only useful when using sub-roots. In all other cases, you can just use include and exclude to filter the files to the ones you care about.

[autarch]

Thinking about it more, I wonder if instead of working_dir.root = "pylib" it should be something like working_dir.chdir_to = "pylib". I think that makes it a bit clearer that this is only changing how the command is executed, not how files are found.

[autarch]

And yet another option:

working_dir = { sub_roots = "pylib", include_all_files = true }

That would be fairly easy to add and I don't think it'd make understanding the config options too complicated.

[oalders]

Nice! Either of those options would work for me. I can't currently think of a reason for needing multiple sub_roots, but there are a lot of linters out there that I'm not familiar with.

[autarch]

After thinking about this a bit I realized that the whole sub_roots thing was just doing too much. If someone wants to restrict the paths that a command processes to a particular sub-tree, we already have include and exclude. So the only other thing it did was change where the command was run to a specific directory.

Given that, I replaced it with working_dir.chdir_to. This will chdir to that directory but will still look for files to process in the entire project. So I think that should meet you use case. I'll have some new test binaries in a few minutes.

[autarch]

Please try out the new build at https://github.com/houseabsolute/precious/actions/runs/3452598158 and let me know.

[oalders]

I tested it out yesterday and this works really well.

[autarch]

I just released v0.4.0 with this change (and a few others) in it.