Skip to content

Commit

Permalink
Allow Input to take validations
Browse files Browse the repository at this point in the history
We allow rules to generate build edges that carry additional
dependencies such as implicit dependencies and build-order dependencies.
This is used when creating in-place format rules to return something
that carries an additional build-order dependency on the formatting
finishing so that all build edges using this as an input will wait on
the formatting to finish before reading the file.

Currently our linting steps (which is just `biome lint` at the moment,
but there is need for a formatting check rule) requires us to pass a
function to the `validations` property - see the `formatAndLint`
function used within `./configure.mjs`. Below is a simplified version,

```js
function formatAndLint(file) {
  return format({
    in: file,
    [validations]: (out) => lint({ in: out }),
  });
}
```

assuming we wanted to format and lint `foo.js` and then copy it to
`out.js`, we would generate something like the following ninja file,

```ninja
build $builddir/lint/foo.js.stamp: lint foo.js || $builddir/format/foo.js.stamp
build $builddir/format/foo.js.stamp: format foo.js |@ $builddir/lint/foo.js.stamp
build out.js: copy foo.js || $builddir/format.foo.js.stamp
```

which guarantees that we only copy `foo.js` once formatting is done, and
whenever we try to format we have a validation on the linting, meaning
it can be done in parallel to the copy.

If we can allow the output to carry `validations` dependencies too then
we can write,

```js
function formatAndLint(file) {
  const formatted = format({ in: file });
  return lint({ in: formatted });
}
```

and we would generate something like the following ninja file,

```ninja
build $builddir/format/foo.js.stamp: format foo.js
build $builddir/lint/foo.js.stamp: lint foo.js || $builddir/format/foo.js.stamp
build out.js: copy foo.js || $builddir/format.foo.js.stamp |@ $builddir/lint/foo.js.stamp
```

noticing that the `out.js` build edge is the thing that carries the
validation rule.  If we had multiple build edges built up from the
return value of `formatAndLint` they would all add both the order-only
dependency on formatting and the validation step on linting.  This is
more verbose than the previous solution, but it is unlikely to cause
issues for the majority of people.  It allows slightly more flexibility
to run ninja on just the format target without requiring linting.  We
also have a nice topological order of all of the edges.
  • Loading branch information
elliotgoodrich committed Mar 21, 2024
1 parent 4c11be1 commit 96c7a57
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
4 changes: 3 additions & 1 deletion packages/core/src/core.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,11 @@ test("Passing all arguments to a `NinjaRule`", () => {
file: "hi",
[implicitDeps]: "implicit1",
[orderOnlyDeps]: "ordered1",
[validations]: ["valid1"],
},
[implicitDeps]: "implicit2",
[orderOnlyDeps]: ["ordered2", "ordered3"],
[validations]: (out: string) => "valid2_" + out,
});
assert.equal(out, "out.txt");
assert.equal(
Expand All @@ -311,7 +313,7 @@ build out.txt | implicitOut_: all in.txt | implicitDeps_ || orderOnlyDeps_ |@ va
description = description_
pool = pool
extra = 123
build foo: all hi | implicit1 implicit2 || ordered1 ordered2 ordered3
build foo: all hi | implicit1 implicit2 || ordered1 ordered2 ordered3 |@ valid1 valid2_foo
`,
);
});
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export type Input<T extends string> =
file: T;
[implicitDeps]?: string | readonly string[];
[orderOnlyDeps]?: string | readonly string[];
[validations]?: string | readonly string[];
};

/**
Expand Down Expand Up @@ -251,6 +252,7 @@ function homogenize(
file?: string | readonly string[];
[implicitDeps]?: string | readonly string[];
[orderOnlyDeps]?: string | readonly string[];
[validations]?: string | readonly string[];
} {
if (input === undefined) {
return {};
Expand All @@ -267,6 +269,7 @@ function homogenize(
file: [] as string[],
[implicitDeps]: [] as string[],
[orderOnlyDeps]: [] as string[],
[validations]: [] as string[],
};
for (let i = 0; i < input.length; ++i) {
const entry = input[i];
Expand All @@ -288,6 +291,13 @@ function homogenize(
out[orderOnlyDeps] = out[orderOnlyDeps].concat(entry[orderOnlyDeps]);
}
}
if (validations in entry) {
if (typeof entry[validations] === "string") {
out[validations].push(entry[validations]);
} else {
out[validations] = out[validations].concat(entry[validations]);
}
}
}
}
return out;
Expand Down Expand Up @@ -565,6 +575,7 @@ export class NinjaBuilder {
file: input,
[implicitDeps]: extraDeps,
[orderOnlyDeps]: extraOrderDeps,
[validations]: extraValidations,
} = homogenize(_in);

// Use a temporary string to not interweave multiple calls on this object
Expand All @@ -590,6 +601,7 @@ export class NinjaBuilder {
) +
concatPaths(
" |@ ",
extraValidations,
buildVariables[validations] === undefined
? undefined
: buildVariables[validations]?.(out),
Expand Down

0 comments on commit 96c7a57

Please sign in to comment.