Skip to content

build: move package.json validation to make recipe #9520

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.

Sign up for GitHub

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

Open
Planeshifter wants to merge 4 commits into
base: develop
Choose a base branch
from
+125 −49
Open

build: move package.json validation to make recipe #9520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f02bd2a
build: move package.json validation to make recipe
Planeshifter Jan 2, 2026
86f42ee
chore: update copyright years
stdlib-bot Jan 3, 2026
be4c8fc
Merge remote-tracking branch 'upstream/develop' into philipp/move-pkg…
stdlib-bot Jan 5, 2026
8f6f49b
Merge remote-tracking branch 'upstream/develop' into philipp/move-pkg…
stdlib-bot Jan 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/lint_changed_files.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ jobs:
run: |
. "$GITHUB_WORKSPACE/.github/workflows/scripts/common/lint_package_json_files" "${{ steps.changed-files.outputs.files }}"

# Validate package.json metadata:
- name: 'Validate package.json metadata'
if: success() || failure()
run: |
make validate-pkg-json FILES="${{ steps.changed-files.outputs.files }}"

# Lint REPL help files...
- name: 'Lint REPL help files'
if: success() || failure()
Expand Down
51 changes: 2 additions & 49 deletions .github/workflows/scripts/common/lint_package_json_files
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#
# @license Apache-2.0
#
# Copyright (c) 2024 The Stdlib Authors.
# Copyright (c) 2026 The Stdlib Authors.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand All @@ -16,7 +16,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# Script to lint package.json files and check/update metadata fields.
# Script to lint package.json files.
#
# Usage: lint_package_json_files file1 [file2 file3 ...]
#
Expand All @@ -32,61 +32,14 @@ root=$(git rev-parse --show-toplevel)
# Define the path to a utility for linting package.json files:
lint_package_json="${root}/lib/node_modules/@stdlib/_tools/lint/pkg-json/bin/cli"

# Define the path to the package name validation tool:
validate_package_names="${root}/lib/node_modules/@stdlib/_tools/lint/pkg-json-names/bin/cli"

# Define paths to utilities for updating package.json metadata fields:
update_package_json_directories="${root}/lib/node_modules/@stdlib/_tools/package-json/scripts/update_directories"
update_package_json_gypfile="${root}/lib/node_modules/@stdlib/_tools/package-json/scripts/update_gypfile"

# Files to process:
files_to_process="$*"

# Initialize needs_changes flag:
needs_changes=0

# Lint package.json files:
files=$(echo "${files_to_process}" | tr ' ' '\n' | awk -F/ '$NF=="package.json"' | tr '\n' ' ' | sed 's/ $//')
if [ -n "${files}" ]; then
echo "Linting package.json files..."
printf '%s' "${files}" | "${lint_package_json}" --split=" "

echo "Validating package names..."
if ! printf '%s' "${files}" | "${validate_package_names}" --split=" "; then
echo "ERROR: Package name validation failed"
needs_changes=1
fi
else
echo "No package.json files to lint."
fi

# Check if metadata fields need to be updated in package.json files of affected packages:
dirs=$(echo "${files_to_process}" | tr ' ' '\n' | \
xargs dirname | \
sed -E 's/\/(benchmark|bin|data|docs|etc|examples|include|lib|scripts|src|test)(\/.*)?$//' | \
sort -u)

echo "Checking package.json files in directories: ${dirs}"
for dir in ${dirs}; do
echo "Checking package.json in ${dir}..."
package_json="${dir}/package.json"
if [ ! -f "${package_json}" ]; then
continue
fi
original_content=$(cat "${package_json}")

"${update_package_json_directories}" "${dir}"
"${update_package_json_gypfile}" "${dir}"

new_content=$(cat "${package_json}")
if [ "$original_content" != "$new_content" ]; then
echo "ERROR: package.json in ${dir} needs updates to directories and/or gypfile fields"
git --no-pager diff "${package_json}"
needs_changes=1
fi
done

# Exit with failure if any needed changes were detected:
if [ $needs_changes -eq 1 ]; then
exit 1
fi
86 changes: 86 additions & 0 deletions lib/node_modules/@stdlib/_tools/package-json/scripts/validate_package_json_files
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
#!/usr/bin/env bash
#
# @license Apache-2.0
#
# Copyright (c) 2026 The Stdlib Authors.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# Script to validate package.json metadata fields.
#
# Usage: validate_package_json_files file1 [file2 file3 ...]
#
# Arguments:
#
# file1 File path.
# file2 File path.
# file3 File path.

# Determine root directory:
root=$(git rev-parse --show-toplevel)

# Define the path to the package name validation tool:
validate_package_names="${root}/lib/node_modules/@stdlib/_tools/lint/pkg-json-names/bin/cli"

# Define paths to utilities for updating package.json metadata fields:
update_package_json_directories="${root}/lib/node_modules/@stdlib/_tools/package-json/scripts/update_directories"
update_package_json_gypfile="${root}/lib/node_modules/@stdlib/_tools/package-json/scripts/update_gypfile"

# Files to process:
files_to_process="$*"

# Initialize needs_changes flag:
needs_changes=0

# Validate package.json package names:
files=$(echo "${files_to_process}" | tr ' ' '\n' | awk -F/ '$NF=="package.json"' | tr '\n' ' ' | sed 's/ $//')
if [ -n "${files}" ]; then
echo "Validating package names..."
if ! printf '%s' "${files}" | "${validate_package_names}" --split=" "; then
echo "ERROR: Package name validation failed"
needs_changes=1
fi
else
echo "No package.json files to validate."
fi

# Check if metadata fields need to be updated in package.json files of affected packages:
dirs=$(echo "${files_to_process}" | tr ' ' '\n' | \
xargs dirname | \
sed -E 's/\/(benchmark|bin|data|docs|etc|examples|include|lib|scripts|src|test)(\/[^@]*)?$//' | \
Copy link
Member Author

@Planeshifter Planeshifter Jan 3, 2026 •
edited by kgryte
Loading

Choose a reason for hiding this comment

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

Note: Replaced (\/.*)? with (\/[^@]*)? to avoid converting paths like ./lib/node_modules/@stdlib/utils/lib to ./, which is undesired (we would want ./lib/node_modules/@stdlib/utils directory in this example). By forbidding @, we can ensure that only lib folders after /@stdlib/ will be stripped.

sort -u)

echo "Checking package.json files in directories: ${dirs}"
for dir in ${dirs}; do
echo "Checking package.json in ${dir}..."
package_json="${dir}/package.json"
if [ ! -f "${package_json}" ]; then
continue
fi
original_content=$(cat "${package_json}")

"${update_package_json_directories}" "${dir}"
"${update_package_json_gypfile}" "${dir}"

new_content=$(cat "${package_json}")
if [ "$original_content" != "$new_content" ]; then
echo "ERROR: package.json in ${dir} needs updates to directories and/or gypfile fields"
git --no-pager diff "${package_json}"
needs_changes=1
fi
done

# Exit with failure if any needed changes were detected:
if [ $needs_changes -eq 1 ]; then
exit 1
fi
14 changes: 14 additions & 0 deletions tools/git/hooks/pre-commit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,20 @@ run_lint() {
else
task_status 'skipped'
fi
add_task 'validate_package_json'
if [[ -z "${skip_package_json}" ]]; then
files=$(echo "${changed_files}" | tr '\n' ' ')
make FILES="${files}" validate-pkg-json > /dev/null >&2
if [[ "$?" -ne 0 ]]; then
task_status 'failed'
echo '' >&2
echo 'package.json metadata out of date' >&2
return 1
fi
task_status 'passed'
else
task_status 'skipped'
fi
# Lint REPL help files...
add_task 'lint_repl_help'
if [[ -z "${skip_repl_help}" ]]; then
Expand Down
17 changes: 17 additions & 0 deletions tools/make/lib/lint/package_json.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ PACKAGE_JSON_LINTER ?= $(TOOLS_PKGS_DIR)/lint/pkg-json/bin/cli
# Define the command-line options to be used when invoking the executable:
PACKAGE_JSON_LINTER_FLAGS ?=

# Define the path for script validating `package.json` metadata:
VALIDATE_PKG_JSON ?= "${TOOLS_PKGS_DIR}/package-json/scripts/validate_package_json_files"


# RULES #

Expand All @@ -38,3 +41,17 @@ lint-pkg-json: $(NODE_MODULES)
$(QUIET) NODE_PATH="$(NODE_PATH)" $(NODE) "$(PACKAGE_JSON_LINTER)" $(PACKAGE_JSON_LINTER_FLAGS) "$(ROOT_DIR)"

.PHONY: lint-pkg-json

#/
# Validates `package.json` metadata associated with a list of files.
#
# @param {string} FILES - list of file paths
#
# @example
# make validate-pkg-json FILES='/foo/lib/index.js /bar/package.json'
#/
validate-pkg-json: $(NODE_MODULES)
Copy link
Member

@kgryte kgryte Jan 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This recipe is somewhat oddly named for a few reasons.

  1. I am not sure we have a validate-* command anywhere else among the make recipes.
  2. The target doesn't follow the convention of appending -files when a recipe is intended to work with a list of files (e.g., lint-javascript-files).

Instead, I would name this target lint-pkg-json-metadata-files.

A few other comments:

  1. In contrast to other linters (e.g., ESLint), this doesn't support the FAST_FAIL environment variable. To do so, we would do something similar to our ESLint recipes where we'd individually iterate over each file.
  2. I think we should have a corresponding lint-pkg-json-metadata recipe which supports globs. Granted, lint-package-json above doesn't work like that atm. This can be punted to a future PR.
  3. I mention (2) because it is odd to me that we need two separate lint commands to fully lint a single package.json file. Because one recipe works on FILES and the other doesn't, we cannot readily create a single lint-pkg-json command which delegates to two more specialized recipes.

Longer term, I think we would be best served by the following targets:

  • lint-pkg-json-schema
  • lint-pkg-json-schema-files
  • lint-pkg-json-metadata
  • lint-pkg-json-metadata-files
  • lint-pkg-json
  • lint-pkg-json-files

where lint-pkg-json then has lint-pkg-json-schema and lint-pkg-json-metadata as prerequisites.

For this PR, I suggest going ahead and at least creating a beachhead by renaming the target and updating the various other downstream consumers in this PR accordingly. I'd also suggest refactoring the recipe to support FAST_FAIL.

Copy link
Member Author

@Planeshifter Planeshifter Jan 5, 2026

Choose a reason for hiding this comment

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

While its true that this is named somewhat differently and lacking the -files suffix that other targets have, my rationale was that it is indeed quite different, because we are not actually linting a list of files. Instead, we resolve all package.json files associated with a list of files and then check whether they need to be updated. Your comment doesn't wrestle with this crucial distinction at all, I think?
FAST_FAIL also doesn't make sense given this context

Copy link
Member

@kgryte kgryte Jan 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, right. This is package-level linting (e.g., if a C implementation is added to a package, then we need to confirm that the package.json is updated accordingly).

In which case, this may be better placed in make/lib/lint/pkgs. Locally, I have a WIP recipe for doing whole package linting with the target make lint-pkgs. Let me go ahead and commit that recipe.

Then, I suggest we rename to lint-pkgs-metadata-files and move the recipe to lint/pkgs. That work for you?

And yes, you're right that FAST_FAIL doesn't map well here.

Copy link
Member

@kgryte kgryte Jan 5, 2026

Choose a reason for hiding this comment

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

Pushed the draft recipe.

Copy link
Member Author

@Planeshifter Planeshifter Jan 5, 2026

Choose a reason for hiding this comment

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

Yeah, that works. Will update PR accordingly.

$(QUIET) echo 'Validating package.json metadata...'
$(QUIET) $(VALIDATE_PKG_JSON) $(FILES)

.PHONY: validate-pkg-json
Loading