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

Allow Input to take validations #20

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

elliotgoodrich
Copy link
Owner

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,

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,

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,

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

and we would generate something like the following ninja file,

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.

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.
@elliotgoodrich elliotgoodrich merged commit 96c7a57 into main Mar 21, 2024
9 checks passed
@elliotgoodrich elliotgoodrich deleted the allow-input-to-take-validations branch March 21, 2024 11:45
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

Successfully merging this pull request may close these issues.

1 participant