joerdav / xc

Markdown defined task runner.
https://xcfile.dev/
MIT License
1.17k stars 27 forks source link

Run dependencies only once #82

Closed brunns closed 1 year ago

brunns commented 1 year ago

Given the following README.md:

## Tasks

### a
```sh
 echo "a"
```
### b
Requires: a
```sh
echo "b"
```
### c
Requires: a
```sh
echo "c"
```
### d
Requires: b, c
```sh
echo "d"
```

Running xc d gives me:

$ xc d
+ echo a
a
+ echo b
b
+ echo a
a
+ echo c
c
+ echo d
d

I'd expect task a to be run only once. Is there a way to get that behavior?

waldyrious commented 1 year ago

If this is implemented, there should be a way to opt in to the exhaustive execution mode. Task a could be something that one wants to execute every time it's indicated as a dependency (e.g. a cleaning/reset step).

brunns commented 1 year ago

I agree - and it would probably be better to make the existing behaviour the default. Maybe something like this in the task?

Run: once
joerdav commented 1 year ago

This makes a lot of sense. It raises the question though, should the task that is depended on decide how many times it should run, or should the task that depends on another task decide:

# task-a
Run: once

echo a

# task-b
Requires: task-a

echo b

OR

# task-a

echo a

# task-b
Requires Once: task-a

echo b

waldyrious commented 1 year ago

IMHO (and without giving it a lot of thought), the requires once syntax feels more natural/intuitive. After all, how should one reconcile the run:once in task-a with the fact that the user can invoke it multiple times in sequence?

On a separate note, it seems like what we care may not be necessarily how many times task-a runs before task-b, but rather whether it must run at any point before task-b (the proposed new behavior) or immediately preceding it (the current behavior).

If this matches your mental models as well, then we might need to adjust the wording from "once" to something else that better reflects this distinction. Maybe something like "parent" vs "ancestor"? or "dependency" vs "immediate/direct-dependency"? Dunno, just spitballing.

joerdav commented 1 year ago

Agree that once may not be the best term. But I think it needs to be blatantly obvious upon reading what it does.

brunns commented 1 year ago

For what it's worth, for the use case which prompted me to suggest this - environment set-up - specifying this on the required task rather than on the requiring would make more sense. That would also avoid the complication of working out what to do if the requiring tasks disagree about how often to run the task.

Either way would work, though, and I'm sure mine isn't the only use case.

joerdav commented 1 year ago

Thinking about it more, it does seem to me like the likelyhood of wanting to define it in the required task is higher. I think for now we should go ahead with that.

Keeping in mind that these tasks should be readable by humans and followed without the use of xc what do you all think of these:

Run: once and Run: always

brunns commented 1 year ago

Looks good to me. I think Run: always should be the default if neither is specified, preserving current behavior, but being able to set it explicitly would be nice for documentation if nothing else.

waldyrious commented 1 year ago

I'm just worried about the distinction between the designation "once" vs. "once per invocation". It seems like the intended behavior is the latter (please confirm), but the intuitive/immediate interpretation IMO would be the former, which might cause some confusion. Or am I overthinking it? :sweat_smile:

brunns commented 1 year ago

Yeah, I was certainly thinking in terms of once per invocation. I think that's what I'd intuitively understand it to mean, too, but perhaps that's just because it was my idea in the first place. ☺

joerdav commented 1 year ago

I believe there's really a couple of angles. What's intuitive to write, and what's intuitive for people approaching without xc.

The most intuitive to write is Run: once as you write it one time in the required task.

The most intuitive for someone reading it would probably be the opposite as Run: once may not mean much to a human. But this could maybe be rectified with a quick description from the author:

## setup

Sets up the environment, should be ran before other steps.

run: once

...script...

joerdav commented 1 year ago

Implemented this in #83

I just need to update the docs before merging.

joerdav commented 1 year ago

@brunns If you want you could install the branch, just double check it's doing what you expected in your use case?

go install github.com/joerdav/xc/cmd/xc@run-once

brunns commented 1 year ago

Thanks, Joe. Yes, that's perfect. With:

## Tasks

### a
Run: once
```sh
 echo "a"
```

### b
Requires: a
```sh
echo "b"
```

### c
Requires: a
```sh
echo "c"
```

### d
Requires: b, c
```sh
echo "d"
```

I now get:

$ xc -V
xc version: v0.2.1-0.20230413075746-d7e06448f0ff (h1:t5gxB27cgDz4NOgMonHOqUAzcT9nwSTQQcd9UVrbuxs=)
$ xc d
+ echo a
a
+ echo b
b
task a already run: skipping
+ echo c
c
+ echo d
d