You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@magpie.apache.org by po...@apache.org on 2026/10/05 16:13:53 UTC

(magpie) branch main updated: fix(dev): follow skills/ symlinks in check-placeholders on BSD grep too (#1531)

This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/magpie.git


The following commit(s) were added to refs/heads/main by this push:
     new 2063f03d fix(dev): follow skills/ symlinks in check-placeholders on BSD grep too (#1531)
2063f03d is described below

commit 2063f03d93e985cac68bbc0f1372c5b043d97ab2
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Mon Oct 5 18:13:46 2026 +0200

    fix(dev): follow skills/ symlinks in check-placeholders on BSD grep too (#1531)
    
    #1522 switched the scan to `grep -R` so it follows the `skills/<name>`
    symlinks into `plugins/`. That holds for GNU grep, but BSD grep (the one
    macOS ships) only follows symlinks under `-R` when `-S` is also given, and
    GNU grep has no `-S`. On macOS the check therefore still skipped every
    skill, and the new test_reports_forbidden_pattern_in_symlinked_skill
    failed in the workspace pytest hook, so every local commit on macOS was
    rejected.
    
    Build the file list once with `find -L`, which follows the links on both,
    and grep that list with `-H` so each match keeps its `skills/<name>/...`
    path.
    
    Generated-by: Claude Opus 5
---
 tools/dev/check-placeholders.sh | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/tools/dev/check-placeholders.sh b/tools/dev/check-placeholders.sh
index 5e2edad7..21fda8e7 100755
--- a/tools/dev/check-placeholders.sh
+++ b/tools/dev/check-placeholders.sh
@@ -115,8 +115,10 @@ INLINE_ALLOW_MARKERS=(
 # Where to look. Only `.md` files under skills + tool adapter docs
 # are scoped; Python sources under `tools/*/src/` and `tools/*/tests/`
 # may legitimately mention Airflow in fixtures and docstrings.
-# Scanned with `grep -R`, not `-r`: every `skills/<name>` is a symlink
-# into `plugins/`, and `-r` skips symlinks it meets while recursing.
+# Every `skills/<name>` is a symlink into `plugins/`. The file list is
+# built with `find -L`, which follows those links on both GNU and BSD;
+# `grep -R` alone does not do it portably (BSD grep, as shipped on
+# macOS, needs `-S` to follow links and GNU grep has no `-S`).
 SCAN_PATHS=(
   "skills"
   "tools"
@@ -164,21 +166,20 @@ main() {
     scan_specs+=( "E:$entry" )
   done
 
+  local -a scan_files=()
+  local scan_file
+  while IFS= read -r -d '' scan_file; do
+    scan_files+=( "$scan_file" )
+  done < <(find -L "${SCAN_PATHS[@]}" -type f -name '*.md' -print0 2>/dev/null)
+
   local spec
   for spec in "${scan_specs[@]}"; do
     local mode="${spec%%:*}"
     local pattern="${spec#*:}"
-    local matches
-    if [[ "$mode" == "F" ]]; then
-      matches=$(grep -RFn \
-        --include='*.md' \
-        "$pattern" \
-        "${SCAN_PATHS[@]}" 2>/dev/null || true)
-    else
-      matches=$(grep -REn \
-        --include='*.md' \
-        "$pattern" \
-        "${SCAN_PATHS[@]}" 2>/dev/null || true)
+    local matches=""
+    if [[ ${#scan_files[@]} -gt 0 ]]; then
+      # -H keeps the file name in each match even if only one file is scanned.
+      matches=$(grep -H -n "-$mode" -e "$pattern" -- "${scan_files[@]}" 2>/dev/null || true)
     fi
 
     if [[ -z "$matches" ]]; then