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

Q/FR: More than one command per job #147

Open
ilyagr opened this issue Jul 2, 2023 · 12 comments
Open

Q/FR: More than one command per job #147

ilyagr opened this issue Jul 2, 2023 · 12 comments

Comments

@ilyagr
Copy link

ilyagr commented Jul 2, 2023

Thanks for bacon, it is very nice!

I don't want to forget running cargo fmt, so I'd like my test job to be the equivalent of cargo fmt && cargo nextest run.

(The actual command I want is cargo fmt && cargo insta run --accept, so both of these can mutate files in the repo).

What's the best way to configure that? It seems like commands can currently only take one command.

Currently, the first workaround that comes to my mind is to create a cargo-run-fmt-and-nextest shell script somewhere in my PATH and use it in my custom job. That's nice, but I think it'd be even nicer if I didn't need to.

For example, the commands option could take either a list of strings or a list of lists of strings. There could be another option that determines whether the command chain is aborted if one of them fails.

@Canop
Copy link
Owner

Canop commented Jul 2, 2023

Regarding running more than one command, a solution, depending on the need, might be to use on_success to chain commands.

For example:

[jobs.go]
command = [
    "cargo", "clippy", "--color", "always",
]
allow_warnings = true
on_success="job:go2"

[jobs.go2]
command = [
    "cargo", "test", "--color", "always",
]
need_stdout = true
allow_warnings = true

[keybindings]
g = "job:go"

With such configuration, hitting g does the tests, then clippy.

There's a problem with cargo fmt though: bacon wasn't at all designed to run such program which never fails or return a list of line related errors.

I'm not yet sure of what would be the best approach for you there.

@ilyagr
Copy link
Author

ilyagr commented Jul 2, 2023

That's very helpful, thanks!

I'll experiment a bit more, I'm becoming worried that something like #138 will become a problem if I run cargo fmt or cargo insta test --accept (which automatically updates test source files to correct the inline snapshots).

BTW, from the department of likely useless technical correction: I've seen cargo fmt report errors if you put error_on_line_overflow = true into rustfmt.toml, but it's admittedly rare.

@ilyagr
Copy link
Author

ilyagr commented Jul 3, 2023

Even before we get to mutating commands, I ran into the problem that the on_success approach does not rerun the first command. So, if my config is

[jobs.msrv-and-clippy]
command = ["cargo", "+1.64", "check", "--workspace", "--all-targets", "--color=always"]
on_success = "job:clippy"
need_stdout = true

I want the msrv-and-clippy job to rerun automatically. However, when I run it once, it switches to the clippy job (as expected). Unfortunately, when something changes in my workspace, only the clippy job gets rerun.

Also, a different I ran into as a part of this is that I entered on-success instead of on_success and couldn't figure out what's wrong for a while. It would be neat if bacon reported an error in this case.

@Larandar
Copy link

It seems to be a subject that has been raised a few times in bacon, and I think that the core problem is that bacon is a single-task runner. Since @Canop re-opened #42, are you looking for a solution?

If I may give my 2 cents, there are a few ways to approach the problem:

  1. The more "hacky"/short-term add on_failed and on_change properties would do things a little like on_success but give a way to express a sort of execution graph.
  2. Rework the executor to have a "context" and remove on_success in favour of dependencies and resolve the corresponding execution graph. It would then rerun everything when there are changes, for example.

IMO, 2 is the more powerful option, but it might be a significant rework, and then the project will open itself to all the feature creeps of cargo-make. Where 1 might not be elegant, it is simple enough and might cover needs expressed until now.

@Canop
Copy link
Owner

Canop commented Oct 23, 2023

A problem is also that use cases aren't really clear, and I'd like to have clarified needs before looking for a solution.

Can you interested people just write as concisely as possible your use case for a sequence of chained tasks in bacon ?

@chrisp60
Copy link

chrisp60 commented Nov 9, 2023

My current need would be to test two specific features sets that are incompatible with each other.

Here is a "wish it would work" toml as example

[jobs.check-ssr]
command = [
    "cargo",
    "clippy",
    "--features=ssr",
    "--no-default-features",
    "--color",
    "always",
]
need_stdout = false
on_success = "job:check-hydrate"

[jobs.check-hydrate]
command = [
    "cargo",
    "clippy",
    "--features=hydrate",
    "--no-default-features",
    "--color",
    "always",
]
need_stdout = false
on_success = "job:previous"

The above loops endlessly

This is my imagined solution

[jobs.check-hydrate]
command = [
    # args,*
]
need_stdout = false
on_change = "job:previous"

essentially stop_on_{success, fail} do action when the watch is triggered again

I am aware there are other build tools to do this, hundreds even. I find bacon to be an easier mental model to develop with.

A split tmux pane is my current solution. Works, but would be nice if it could be condensed to one.

Other build tools don't always produce in a format that bacon can parse and summarize.

maybe I can look into a solution if pointed in the right direction within the repo?

@Canop
Copy link
Owner

Canop commented Nov 9, 2023

Your need could be seen as to have the warnings & errors of two different cargo executions merged into only one report, right ?

@chrisp60
Copy link

chrisp60 commented Nov 9, 2023

Your need could be seen as to have the warnings & errors of two different cargo executions merged into only one report, right ?

Concisely yes,
I am indifferent to them running parallel, forked, or in sequence when fails are encountered individually.

Just 1+ cargo commands in one report, redo on change

Sorry to keep editing, I just realize there is a lot of nuance to the issue

@Canop
Copy link
Owner

Canop commented Nov 9, 2023

Sorry to keep editing, I just realize there is a lot of nuance to the issue

I know. That's why I don't rush to a "solution" but try to grasp the whole problem before.

@Canop
Copy link
Owner

Canop commented Nov 19, 2023

Let's have a look at the "depends of" logic and how it could be applied to bacon.

Considering "C depends of A and B", I don't think we can support a pure make-like approach with C being run only when A and B are verified to be error or warning free.

Because that would imply running A, B and C on every file change even if A and B were previously OK. This isn't going to be practical given that compilation times are already a common problem in Rust.

For the same reason, I don't think it's a good idea to run several commands, wait for their results, and then display the combined result. This would be too slow in big projects.

What I think would be best would be the ability to launch not just jobs but job sets (externally, they would be called jobs, no need to make the job set concept visible).

Such a job set would be defined as a list of jobs [A, B, C]:

[jobs.check-all-the-things]
jobs = ["chek", "clippy", "windows-check", "ssr-feature"]

The execution of this job set would be like this:

  • initially, all jobs are flagged dirty
  • the file change detector is based on the union of of directories and files of all simple jobs. When a file change is detected, all jobs are flagged dirty (there could be an optimization maybe to flag only the relevant ones)
  • the initial current job is the first one (A)
  • on change, only the current job is ran
  • on success of a job, it's no more dirty, and another dirty job becomes the current one and is immediately started
  • when all jobs are successful, and none is dirty, the job set is successful (and there's no execution)

It think this is the right approach:

  • it doesn't involve longer waits, or excessive computations
  • it's simple enough to understand and configure

Do you see use cases which wouldn't fit well with this approach.

@Larandar
Copy link

Larandar commented Dec 1, 2023

I think it is indeed the right solution; bacon is by design simple, and supporting a complete DAG like cargo-make seems overkill. This would fit all the usecases I currently see for myself, but attention must be taken for jobs that modify the src themselves ([cargo-fmt + autofix -> check -> clippy -> test -> run])

@chrisp60
Copy link

Do you see use cases which wouldn't fit well with this approach.

Unable to think of anything that wouldn't work with this idea (for myself at least). I would imagine anything beyond a set of jobs would be crossing over into a much more complex build system.

I appreciate Bacon being open to a feature like this, it would certainly be a convenience more than a necessity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants