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

Rationalise DCR's env checking scripts #9400

Merged
merged 10 commits into from
Nov 8, 2023
16 changes: 8 additions & 8 deletions dotcom-rendering/makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ export SHELL := /usr/bin/env bash
# messaging #########################################

define log
@node scripts/env/log $(1)
@node ../scripts/log $(1)
endef

define warn
@node scripts/env/log $(1) warn
@node ../scripts/log $(1) warn
endef

# deployment #########################################
Expand Down Expand Up @@ -134,9 +134,11 @@ lint: clean-dist install
$(call log, "checking for lint errors")
@yarn lint

lint-project:
lint-project: check-env
$(call log, "linting project")
@node ../scripts/check-node-versions.mjs
@node scripts/check-node-versions.mjs
@node scripts/env/check-deps.js
@node scripts/env/check-files.js

stylelint: clean-dist install
$(call log, "checking for style lint errors")
Expand Down Expand Up @@ -181,10 +183,8 @@ validate-build: # private

check-env: # private
$(call log, "checking environment")
@node scripts/env/check-node.js
@node scripts/env/check-yarn.js
@node scripts/env/check-deps.js
@node scripts/env/check-files.js
@cd .. && scripts/env/check-node
@cd .. && scripts/env/check-yarn

clear: # private
@clear
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
import { readFile } from 'node:fs/promises';
import { dirname, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';
import { log, warn } from '../dotcom-rendering/scripts/env/log.js';
import { log, warn } from '../../scripts/log.js';

const __dirname = dirname(fileURLToPath(import.meta.url));

process.chdir(resolve(__dirname, '..'));

const nvmrc = (await readFile('.nvmrc', 'utf-8'))
const nvmrc = (await readFile('../.nvmrc', 'utf-8'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure this should live inside dotcom-rendering as it’s looking for files in both workspaces and in the root.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a DCR concern though right? it's looking in the root for the root version, and making sure that DCR-related files (which AR files are, really, i think?) are in sync with the repo more generally.

should it be the responsibility of the root to ensure that apps in the workspace stay in sync with the root?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my mind the root is a good place for scripts that span all workspaces, but I agree that it should be a workspace concern to stay in sync with the root.

I’ll defer to anyone in @guardian/dotcom-platform with a stronger or better articulated view.

Copy link
Member Author

@sndrs sndrs Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged, but if anyone prefers to move it, please do

// We don’t care about leading or trailing whitespace
.trim();

Expand All @@ -30,15 +30,15 @@ if (!nodeVersion) {
const requiredNodeVersionMatches =
/** @type {const} @satisfies {ReadonlyArray<{filepath: string, pattern: RegExp}>}*/ ([
{
filepath: 'dotcom-rendering/Containerfile',
filepath: 'Containerfile',
pattern: /^FROM node:(.+)-alpine$/m,
},
{
filepath: 'dotcom-rendering/scripts/deploy/riff-raff.yaml',
filepath: 'scripts/deploy/riff-raff.yaml',
pattern: /^ +Recipe: dotcom-rendering.*-node-(\d+\.\d+\.\d+)$/m,
},
{
filepath: 'apps-rendering/riff-raff.yaml',
filepath: '../apps-rendering/riff-raff.yaml',
pattern: /^ +Recipe: .+-mobile-node(\d+\.\d+\.\d+).*$/m,
},
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import path from 'node:path';
import * as url from 'node:url';
import cpy from 'cpy';
import execa from 'execa';
import { log, warn } from '../env/log.js';
import { log, warn } from '../../../scripts/log.js';

const dirname = url.fileURLToPath(new URL('.', import.meta.url));
const target = path.resolve(dirname, '../..', 'target');
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/scripts/env/check-deps.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const fs = require('node:fs');
const lockfile = require('@yarnpkg/lockfile');
const { warn, log } = require('../../../scripts/log');
const pkg = require('../../package.json');
const { warn, log } = require('./log');

if (pkg.devDependencies) {
warn('Don’t use devDependencies');
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/scripts/env/check-files.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const execa = require('execa');
const { warn } = require('./log');
const { warn } = require('../../../scripts/log');

execa('find', ['src', '-type', 'f', '-name', '*index*.ts*'])
.then(({ stdout }) => {
Expand Down
46 changes: 0 additions & 46 deletions dotcom-rendering/scripts/env/check-node.js

This file was deleted.

32 changes: 0 additions & 32 deletions dotcom-rendering/scripts/env/check-yarn.js

This file was deleted.

37 changes: 0 additions & 37 deletions dotcom-rendering/scripts/env/ensure.js

This file was deleted.

4 changes: 2 additions & 2 deletions dotcom-rendering/scripts/gen-stories/get-stories.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import { mkdirSync, readFileSync, writeFileSync } from 'node:fs';
import { dirname, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';
import { log, success, warn } from '../env/log.js';
import { log, success, warn } from '../../../scripts/log.js';

const STORIES_PATH = resolve(
dirname(fileURLToPath(new URL(import.meta.url))),
Expand Down Expand Up @@ -132,7 +132,7 @@ ${storyVariableName}.args = { config: ${JSON.stringify(config)} };
${storyVariableName}.decorators = [lightDecorator({
display: ArticleDisplay.${displayName},
design: ArticleDesign.${designName},
theme: {...ArticleSpecial, ...Pillar}.${theme.replace("Pillar", "")},
theme: {...ArticleSpecial, ...Pillar}.${theme.replace('Pillar', '')},
})];
`;
};
Expand Down
2 changes: 1 addition & 1 deletion dotcom-rendering/scripts/perf/perf-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const execa = require('execa');
const { warn, log } = require('../env/log');
const { warn, log } = require('../../../scripts/log');

const run = async () => {
try {
Expand Down
86 changes: 86 additions & 0 deletions scripts/env/check-node
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
#!/usr/bin/env bash

# Check whether the current node version matches the .nvmrc version, and offer some help if not.

in_terminal() { test -t 1; }

strip_colors() {
local text="$1"
echo "$text" | sed -e $'s/\x1b\[[0-9;]*m//g'
}

log_with_color() {
local text="$1"

# Check if output is a terminal and supports color
if in_terminal && [[ $(tput colors) -ge 8 ]]; then
# Terminal supports color, print the text as it is
echo -e "$text"
else
# Terminal does not support color, strip color codes and print
echo "$(strip_colors "$text")"
fi
}

blue='\033[0;34m'
red='\033[0;31m'
dim='\033[2m'
bold='\033[1m'
reset='\033[0m'

# get the node version from .nvmrc
nvmrc_contents=$(cat .nvmrc)

# check that it's a valid version (matches x.y.z)
nvmrc_version_pattern="^[0-9]+\.[0-9]+\.[0-9]+$"
if [[ $nvmrc_contents =~ $nvmrc_version_pattern ]]; then
nvmrc_version=${BASH_REMATCH[0]}
else
log_with_color "${red}The Node version in .nvmrc is invalid${reset}"
log_with_color "${dim}It should match the pattern 'x.y.z'.${reset}"
# exit with failure
exit 1
fi

# Now we can check if the current version of Node matches the .nvmrc version

# is _any_ node available?
if [[ -x "$(command -v node)" ]]; then
node_version=$(node -v)
node_version=${node_version:1} # remove the "v" in "v1.2.3"
fi

# check the version we're using
# note that `node_version` will be empty if node wan't available above
if [ "$node_version" == "$nvmrc_version" ]; then
# we're using the correct version of node
log_with_color "${dim}Using Node ${blue}$node_version${reset}"

# we're done, exit with success
exit 0
fi

# If we got here, we're using the wrong version of node, or There Is No Node.
# Try to help people load the correct version:
if in_terminal; then
log_with_color "${red}This project requires Node v$nvmrc_version${reset}"
if [[ -x "$(command -v fnm)" ]]; then
log_with_color "${dim}Run ${blue}fnm use${reset}${dim} to switch to the correct version.${reset}"
log_with_color "${dim}See ${blue}${dim}https://github.com/Schniz/fnm#shell-setup${reset}${dim} to automate this.${reset}"
elif [[ -x "$(command -v asdf)" ]]; then
log_with_color "${dim}Run ${blue}asdf install${reset}${dim} to switch to the correct version.${reset}"
elif [[ -x "$(which nvm)" ]]; then
log_with_color "${dim}Run ${blue}nvm install${reset}${dim} to switch to the correct version.${reset}"
else
log_with_color "${dim}Consider using ${bold}fnm${reset}${dim} to manage Node versions:${reset} ${blue}https://github.com/Schniz/fnm#installation${reset}${dim}.${reset}"
fi
else
# not in a terminal, so v possible we're running a husky script in a git gui
echo "Could not find Node v$nvmrc_version."
echo "You may need to load your Node version manager in a ~/.huskyrc file."
echo "See https://typicode.github.io/husky/troubleshooting.html#command-not-found."
fi

# exit with failure
exit 1

28 changes: 28 additions & 0 deletions scripts/env/check-yarn
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#!/usr/bin/env node

/**
* Check that yarn is available. If not, ask the user to run `corepack enable`
* which will provide it.
*
* Note that any yarn@>1 will do, since it will defer to
* the copy in that lives in .yarn/releases (which is the version we want to
* use).
*/

// no external deps can be used here, because this runs before deps are installed
const { execSync } = require('child_process');
const { stdout } = require('process');
const { colourise, log, warn } = require('../log');

// we can't use chalk, because this runs before deps are installed
// https://gist.github.com/fnky/458719343aabd01cfb17a3a4f7296797
const reset = '\x1b[0m';
const blue = '\x1b[34m';

try {
const yarnVersion = execSync('yarn -v', { encoding: 'utf-8' });
log(`Using yarn ${colourise(reset, colourise(blue, yarnVersion.trim()))}`);
} catch (e) {
warn(`Could not find yarn. Please run 'corepack enable'.`);
process.exit(1);
}
9 changes: 7 additions & 2 deletions dotcom-rendering/scripts/env/log.js → scripts/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@ const capitalize = (str) =>

// we could use chalk, but this saves needing to pre-install it
// if this is a first run
// https://gist.github.com/fnky/458719343aabd01cfb17a3a4f7296797
const red = '\x1b[31m';
const yellow = '\x1b[33m';
const green = '\u001b[32m';
const green = '\x1b[32m';
const dim = '\x1b[2m';
const reset = '\x1b[0m';

const colourise = (colour, str) =>
process.stdout.isTTY ? `${colour}${str}${reset}` : str;

const logIt = (messages = [], color = dim) => {
console.log(`${color}%s${reset}`, capitalize(messages.join('\n')));
console.log(colourise(color, capitalize(messages.join('\n'))));
};

const log = (...messages) => logIt(messages);
Expand Down Expand Up @@ -42,4 +46,5 @@ module.exports = {
warn,
prompt,
success,
colourise,
};
Loading