simplesurance / baur

An incremental task runner for mono repositories.
GNU General Public License v2.0
362 stars 11 forks source link

Add command to diff the inputs of 2 builds #120

Closed fho closed 4 years ago

fho commented 4 years ago

Add a command that shows the list of inputs that are different between two builds:

The usage could be like: baur diff inputs <APP-NAME>.<TASK-NAME>|<TASK-RUN-ID> <APP-NAME>.<TASK-NAME>|<TASK-RUN-ID>

Parameters to support:

Optional: support to specify a syntax like gits HEAD^^ to show the last build or last-X build of an application. Idea:

Source: https://github.com/simplesurance/baur/issues/117#issuecomment-613286602_

dil-spayne commented 4 years ago

@fho I've started putting some test case scenarios together for this work and I would appreciate your input. Can you please review the following and help me answer the questions in the Comments column. Also, do you have a preferred format for the Output?

Data Setup

Application.Task RunID Inputs
app_one.build n/a, current files file_one - sha384:321
file_two - sha384:456
file_four - sha384:987
app_one.build 1 file_one - sha384:123
app_one.build 2 file_one - sha384:321
file_two - sha384:456
file_three - sha384:789
app_two.compile n/a, current files file_one - sha384:123
file_two - sha384:456
app_two.compile 3 file_one - sha384:123
file_two - sha384:456

Test Cases

Argument One Argument Two Output Exit code
app_one.build   Error: accepts 2 args, received x 1
app_one.build^   Error: accepts 2 args, received x 1
1 Error: accepts 2 args, received x 1
app_one app_two.compile  Error: app_one does not specify a task 1
app_one.build^^^ 1 Error: app_one.build^^^ does not exist, (only found 2 recorded runs in the database) 1
app_one.build app_one.build Error: app_one.build and app_one.build refer to the same task-run 1
app_one.build^^ 1 Error: app_one.build^^ and 1 refer to the same task-run 1
app_one.build 99 Error: task-run 99 does not exist 1
unknown.thing 1 Error: task does not exist 1
app_one.build app_one.build^ 
StatePathDigest1Digest2
+file_threesha384:789
-file_foursha384:987
2
app_one.build app_one.build^^ 
StatePathDigest1Digest2
Dfile_onesha384:321sha384:123
-file_twosha384:456
-file_foursha384:987
2
app_one.build 1
StatePathDigest1Digest2
Dfile_onesha384:321sha384:123
-file_twosha384:456
-file_foursha384:987
2
1 2
StatePathDigest1Digest2
Dfile_onesha384:123sha384:321
+file_twosha384:456
+file_threesha384:789
2
app_one.build app_two.compile
StatePathDigest1Digest2
Dfile_onesha384:321sha384:123
-file_foursha384:987
2
app_one.build^ app_two.compile
StatePathDigest1Digest2
Dfile_onesha384:321sha384:123
-file_threesha384:789
2
app_two.compile 3
StatePathDigest1Digest2
0
fho commented 4 years ago

@StephenDiligent

My usage description seemed to have been unclear. With the introduction of tasks, it also changed a bit (I'll also update the description.).

The syntax<APP-NAME>.<TASK-NAME>|<TASK-RUN-ID> should mean either to specify the task-name of an app or a task-run-id.

For example:

baur diff inputs calc.build 1 # compare the current inputs of the build task of the calc application with the ones of task-run 1
baur diff inputs 1 2 # compare the inputs of run 1 and 2
baur diff inputs calc.build shop.test # compare the current inputs of the build task of the calc app with the current inputs of the test task of the shop app

@fho I've started putting some test case scenarios together for this work and I would appreciate your input. Can you please review the following and help me answer the questions in the Comments column.

app_one app_two^^ ? 1 How do we handle “run doesn’t exist”? app_one|3 app_two|3 ? 1 How do we handle “run doesn’t exist for either app”? app_one app_three ? 1 How do we handle “app doesn’t exist”?

When an app or run-id that was specified as parameter does not exist, always print an error message and exit. We don't not need to do anything in the case. The user provided parameter invalid values.

Also, do you have a preferred format for the Output?

I had a format like the following in mind: $ baur diff inputs random.build 1:

State   Path            Digest1         Digest2
D       main.go         sha384:123      sha384:12345
+       test.go                         sha384:2950
-       hello.go        sha384:6891

The State column indicates the type of change:

The Digest1 is the digest from the inputs of the first parameter, Digest2 of the second parameter.

We could color the lines according to the status on the console. This format could also be printed in csv format (--csv) when using the table formatter.

What do you think?

Thanks for your help :tada:.

dil-spayne commented 4 years ago

@StephenDiligent

My usage description seemed to have been unclear. With the introduction of tasks, it also changed a bit (I'll also update the description.).

The syntax<APP-NAME>.<TASK-NAME>|<TASK-RUN-ID> should mean either to specify the task-name of an app or a task-run-id.

For example:

baur diff inputs calc.build 1 # compare the current inputs of the build task of the calc application with the ones of task-run 1
baur diff inputs 1 2 # compare the inputs of run 1 and 2
baur diff inputs calc.build shop.test # compare the current inputs of the build task of the calc app with the current inputs of the test task of the shop app

@fho I've started putting some test case scenarios together for this work and I would appreciate your input. Can you please review the following and help me answer the questions in the Comments column.

app_one app_two^^ ? 1 How do we handle “run doesn’t exist”? app_one|3 app_two|3 ? 1 How do we handle “run doesn’t exist for either app”? app_one app_three ? 1 How do we handle “app doesn’t exist”?

When an app or run-id that was specified as parameter does not exist, always print an error message and exit. We don't not need to do anything in the case. The user provided parameter invalid values.

Also, do you have a preferred format for the Output?

I had a format like the following in mind: $ baur diff inputs random.build 1:

State   Path            Digest1         Digest2
D       main.go         sha384:123      sha384:12345
+       test.go                         sha384:2950
-       hello.go        sha384:6891

The State column indicates the type of change:

  • D - different digest
  • + the inputs of the run/task specified by the second parameter has a file that is not part of the inputs of the run/task of specified by the first parameter
  • - analogous, the inputs is part of the task/run specified by the first parameter but not of the task/run specified by the second parameter

The Digest1 is the digest from the inputs of the first parameter, Digest2 of the second parameter.

We could color the lines according to the status on the console. This format could also be printed in csv format (--csv) when using the table formatter.

What do you think?

Thanks for your help 🎉 .

Thank you for your feedback, it all makes sense and should simply the implementation that I have been working on. I have updated the test cases above, do you mind taking another look at them? Using the existing table formatter should work well for this command :+1:

fho commented 4 years ago

I have updated the test cases above, do you mind taking another look at them?

We misunderstand regarding the caret (^ syntax: It is meant to work like in git. You still have to specify always 2 parameters to baur diff inputs. app_one.build^ - references the last recorded run of the task app_one.build^ - resolves the last recoded run before the last

That means for you examples:

Argument One Argument Two Output Exit code
app_one.build^ 1 show diff between run 2 and run 1 2
app_one.build^^ 1 Error: app_one.build^^ and 1 refer to the same task-run 1
app_one.build^^^ 1 Error: app_one.build^^^ does not exist, (only found 2 recorded runs in the database) 1
app_one.build^ app_two.compile show diff between run 2 and current files of app_two.compile 2
app_one.build^ Error: 2 parameter must be specified 1
app_one.build^^ Error: 2 parameter must be specified 1

The first implementation could also be done without the caret syntax. It makes it only more user-friendly, we can add that syntax in a follow-up PR.

I would also suggest that was use different exit codes when baur exits because of an error and when diff inputs found differences. Exit code for error:1, If differences were found, exit code:2.

fho commented 4 years ago

@StephenDiligent btw, before you stumple over it: :-) The table formatter currently does not work with colors currently. It calculates the column width wrongly in some cases when the string contains ANSCI code sequencens (https://github.com/simplesurance/baur/issues/205), easiest is to not use colors for now together with the formatter.