-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(version-scanner): support target list inputs via --targets #17478
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -137,7 +137,9 @@ def load_config(self) -> List[Dict[str, str]]: | |||||||||||||||||||||||||||||
| resolved_pattern = template.strip().format(**self.variables) | ||||||||||||||||||||||||||||||
| resolved_rules.append({ | ||||||||||||||||||||||||||||||
| "name": name, | ||||||||||||||||||||||||||||||
| "pattern": resolved_pattern | ||||||||||||||||||||||||||||||
| "pattern": resolved_pattern, | ||||||||||||||||||||||||||||||
| "dependency": self.dependency, | ||||||||||||||||||||||||||||||
| "version": self.version | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
| except KeyError as e: | ||||||||||||||||||||||||||||||
| print(f"Warning: Missing variable for interpolation in rule {name}: {e}", file=sys.stderr) | ||||||||||||||||||||||||||||||
|
|
@@ -178,7 +180,9 @@ def scan_file(file_path: str, compiled_rules: List[Dict[str, re.Pattern]]) -> Li | |||||||||||||||||||||||||||||
| "rule_name": rule["name"], | ||||||||||||||||||||||||||||||
| "line_number": line_num, | ||||||||||||||||||||||||||||||
| "matched_string": match.group(0).strip(), | ||||||||||||||||||||||||||||||
| "context_line": line.strip() | ||||||||||||||||||||||||||||||
| "context_line": line.strip(), | ||||||||||||||||||||||||||||||
| "dependency": rule.get("dependency", ""), | ||||||||||||||||||||||||||||||
| "version": rule.get("version", "") | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
| except IOError as e: | ||||||||||||||||||||||||||||||
| print(f"Warning: Could not read file {file_path}: {e}", file=sys.stderr) | ||||||||||||||||||||||||||||||
|
|
@@ -465,10 +469,11 @@ def read_package_file(file_path: str) -> List[str]: | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def scan_repository( | ||||||||||||||||||||||||||||||
| root_path: str, | ||||||||||||||||||||||||||||||
| rules: List[Dict[str, str]], | ||||||||||||||||||||||||||||||
| rules: List[Dict[str, Any]], | ||||||||||||||||||||||||||||||
| target_packages: List[str] = None, | ||||||||||||||||||||||||||||||
| ignore_dirs: List[str] = None, | ||||||||||||||||||||||||||||||
| version_string: str = None | ||||||||||||||||||||||||||||||
| version_string: str = None, | ||||||||||||||||||||||||||||||
| targets: List[Tuple[str, str]] = None | ||||||||||||||||||||||||||||||
| ) -> List[Dict[str, Any]]: | ||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||
| Scans the repository directory tree applying resolved regex patterns to files. | ||||||||||||||||||||||||||||||
|
|
@@ -487,21 +492,30 @@ def scan_repository( | |||||||||||||||||||||||||||||
| performs a full recursive scan of the repository. | ||||||||||||||||||||||||||||||
| ignore_dirs: Optional list of directory names or glob-like files to ignore (case-insensitive). | ||||||||||||||||||||||||||||||
| version_string: Optional target version string (e.g. "3.7") to scan for in filenames. | ||||||||||||||||||||||||||||||
| targets: Optional list of (dependency, version) tuples. | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||||
| A list of dictionaries detailing each match: 'file_path', 'repo_path', | ||||||||||||||||||||||||||||||
| 'package_name', 'rule_name', 'line_number', 'matched_string', 'context_line'. | ||||||||||||||||||||||||||||||
| A list of dictionaries detailing each match. | ||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||
| ignore_lower = {i.lower() for i in ignore_dirs} if ignore_dirs else set() | ||||||||||||||||||||||||||||||
| results = [] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| filename_targets = [] | ||||||||||||||||||||||||||||||
| if targets: | ||||||||||||||||||||||||||||||
| filename_targets = targets | ||||||||||||||||||||||||||||||
| elif version_string: | ||||||||||||||||||||||||||||||
| dep = rules[0].get("dependency") if rules else None | ||||||||||||||||||||||||||||||
| filename_targets = [(dep, version_string)] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Compile patterns once here | ||||||||||||||||||||||||||||||
| compiled_rules = [] | ||||||||||||||||||||||||||||||
| for rule in rules: | ||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||
| compiled_rules.append({ | ||||||||||||||||||||||||||||||
| "name": rule["name"], | ||||||||||||||||||||||||||||||
| "pattern": re.compile(rule["pattern"], re.IGNORECASE) | ||||||||||||||||||||||||||||||
| "pattern": re.compile(rule["pattern"], re.IGNORECASE), | ||||||||||||||||||||||||||||||
| "dependency": rule.get("dependency", ""), | ||||||||||||||||||||||||||||||
| "version": rule.get("version", "") | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
| except re.error as e: | ||||||||||||||||||||||||||||||
| print(f"Error compiling regex for rule {rule['name']}: {e}", file=sys.stderr) | ||||||||||||||||||||||||||||||
|
|
@@ -541,13 +555,16 @@ def scan_repository( | |||||||||||||||||||||||||||||
| matches = scan_file(file_path, compiled_rules) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Add filename match if applicable | ||||||||||||||||||||||||||||||
| if version_string and version_string in file: | ||||||||||||||||||||||||||||||
| matches.append({ | ||||||||||||||||||||||||||||||
| "rule_name": "filename_match", | ||||||||||||||||||||||||||||||
| "line_number": 0, | ||||||||||||||||||||||||||||||
| "matched_string": version_string, | ||||||||||||||||||||||||||||||
| "context_line": f"Filename contains {version_string}" | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
| for dep, ver in filename_targets: | ||||||||||||||||||||||||||||||
| if ver and ver in file: | ||||||||||||||||||||||||||||||
| matches.append({ | ||||||||||||||||||||||||||||||
| "rule_name": "filename_match", | ||||||||||||||||||||||||||||||
| "line_number": 0, | ||||||||||||||||||||||||||||||
| "matched_string": ver, | ||||||||||||||||||||||||||||||
| "context_line": f"Filename contains {ver}", | ||||||||||||||||||||||||||||||
| "dependency": dep or "", | ||||||||||||||||||||||||||||||
| "version": ver | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Compute display path and package name | ||||||||||||||||||||||||||||||
| rel_file_path = os.path.relpath(file_path, root_path) | ||||||||||||||||||||||||||||||
|
|
@@ -576,6 +593,46 @@ def scan_repository( | |||||||||||||||||||||||||||||
| return results | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def parse_targets(targets_input: str) -> List[Tuple[str, str]]: | ||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||
| Parses a targets input (file path or inline YAML/JSON string) into a list of (dependency, version) tuples. | ||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||
| raw_targets = {} | ||||||||||||||||||||||||||||||
| content = targets_input | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Check if the input is a file path | ||||||||||||||||||||||||||||||
| if os.path.exists(targets_input): | ||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||
| with open(targets_input, 'r', encoding='utf-8') as f: | ||||||||||||||||||||||||||||||
| content = f.read() | ||||||||||||||||||||||||||||||
| except PermissionError: | ||||||||||||||||||||||||||||||
| print(f"Error: Permission denied reading targets file: {targets_input}", file=sys.stderr) | ||||||||||||||||||||||||||||||
| sys.exit(1) | ||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||
| print(f"Error reading targets file {targets_input}: {e}", file=sys.stderr) | ||||||||||||||||||||||||||||||
| sys.exit(1) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||
| raw_targets = yaml.safe_load(content) | ||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||
| print(f"Error parsing targets YAML/JSON content: {e}", file=sys.stderr) | ||||||||||||||||||||||||||||||
| sys.exit(1) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if not isinstance(raw_targets, dict): | ||||||||||||||||||||||||||||||
| print("Error: Targets input must resolve to a JSON object or YAML mapping", file=sys.stderr) | ||||||||||||||||||||||||||||||
| sys.exit(1) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| targets = [] | ||||||||||||||||||||||||||||||
| for dep, versions in raw_targets.items(): | ||||||||||||||||||||||||||||||
| if isinstance(versions, list): | ||||||||||||||||||||||||||||||
| for v in versions: | ||||||||||||||||||||||||||||||
| targets.append((str(dep), str(v))) | ||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||
| targets.append((str(dep), str(versions))) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return targets | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def main(): | ||||||||||||||||||||||||||||||
| script_dir = os.path.dirname(os.path.abspath(__file__)) | ||||||||||||||||||||||||||||||
| default_config = os.path.join(script_dir, "regex_config.yaml") | ||||||||||||||||||||||||||||||
|
|
@@ -586,16 +643,19 @@ def main(): | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| parser.add_argument( | ||||||||||||||||||||||||||||||
| "-d", "--dependency", | ||||||||||||||||||||||||||||||
| required=True, | ||||||||||||||||||||||||||||||
| help="Name of the dependency (e.g., python, protobuf)" | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| parser.add_argument( | ||||||||||||||||||||||||||||||
| "-v", "--version", | ||||||||||||||||||||||||||||||
| required=True, | ||||||||||||||||||||||||||||||
| help="Specific version to search for (e.g., 3.7, 4.25.8)" | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| parser.add_argument( | ||||||||||||||||||||||||||||||
| "--targets", | ||||||||||||||||||||||||||||||
| help="Path to a YAML/JSON targets file, or an inline YAML/JSON string (e.g. 'python: [3.8, 3.9]')" | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| parser.add_argument( | ||||||||||||||||||||||||||||||
| "-p", "--path", | ||||||||||||||||||||||||||||||
| default=".", | ||||||||||||||||||||||||||||||
|
|
@@ -659,6 +719,25 @@ def main(): | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| args = parser.parse_args() | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Validation of required inputs | ||||||||||||||||||||||||||||||
| has_single_target = bool(args.dependency and args.version) | ||||||||||||||||||||||||||||||
| has_targets_list = bool(args.targets) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if not (has_single_target or has_targets_list): | ||||||||||||||||||||||||||||||
| parser.error("Must specify either (-d/--dependency AND -v/--version) OR (--targets)") | ||||||||||||||||||||||||||||||
| if has_single_target and has_targets_list: | ||||||||||||||||||||||||||||||
| parser.error("Cannot specify both single target (-d/-v) and targets list (--targets)") | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| targets = [] | ||||||||||||||||||||||||||||||
| if has_targets_list: | ||||||||||||||||||||||||||||||
| targets = parse_targets(args.targets) | ||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||
| targets = [(args.dependency, args.version)] | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if not targets: | ||||||||||||||||||||||||||||||
| print("Error: No targets resolved to scan.", file=sys.stderr) | ||||||||||||||||||||||||||||||
| sys.exit(1) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Resolve target packages if filtering is requested | ||||||||||||||||||||||||||||||
| target_packages = [] | ||||||||||||||||||||||||||||||
| if args.package: | ||||||||||||||||||||||||||||||
|
|
@@ -670,7 +749,12 @@ def main(): | |||||||||||||||||||||||||||||
| elif args.package_file: | ||||||||||||||||||||||||||||||
| target_packages = read_package_file(args.package_file) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| print(f"Starting scan for dependency: {args.dependency} version: {args.version}") | ||||||||||||||||||||||||||||||
| if has_targets_list: | ||||||||||||||||||||||||||||||
| print("Starting scan for multiple targets:") | ||||||||||||||||||||||||||||||
| for dep, ver in targets: | ||||||||||||||||||||||||||||||
| print(f" - {dep}: {ver}") | ||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||
| print(f"Starting scan for dependency: {args.dependency} version: {args.version}") | ||||||||||||||||||||||||||||||
| print(f"Root path: {args.path}") | ||||||||||||||||||||||||||||||
| print("Targets to scan:") | ||||||||||||||||||||||||||||||
| if target_packages: | ||||||||||||||||||||||||||||||
|
|
@@ -681,8 +765,10 @@ def main(): | |||||||||||||||||||||||||||||
| print(f"Using config: {args.config}") | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Load and resolve rules | ||||||||||||||||||||||||||||||
| config_manager = ConfigManager(args.config, args.dependency, args.version) | ||||||||||||||||||||||||||||||
| rules = config_manager.load_config() | ||||||||||||||||||||||||||||||
| rules = [] | ||||||||||||||||||||||||||||||
| for dep, ver in targets: | ||||||||||||||||||||||||||||||
| config_manager = ConfigManager(args.config, dep, ver) | ||||||||||||||||||||||||||||||
| rules.extend(config_manager.load_config()) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
@@ -695,7 +781,14 @@ def main(): | |||||||||||||||||||||||||||||
| print(f"Loaded {len(ignore_dirs)} ignore patterns from {ignore_file_path}") | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| # Scan repository | ||||||||||||||||||||||||||||||
| all_matches = scan_repository(args.path, rules, target_packages, ignore_dirs, version_string=args.version) | ||||||||||||||||||||||||||||||
| all_matches = scan_repository( | ||||||||||||||||||||||||||||||
| args.path, | ||||||||||||||||||||||||||||||
| rules, | ||||||||||||||||||||||||||||||
| target_packages, | ||||||||||||||||||||||||||||||
| ignore_dirs, | ||||||||||||||||||||||||||||||
| version_string=(None if has_targets_list else args.version), | ||||||||||||||||||||||||||||||
| targets=targets | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| print(f"\nFound {len(all_matches)} matches.") | ||||||||||||||||||||||||||||||
| display_matches = all_matches if args.stdout else all_matches[:10] | ||||||||||||||||||||||||||||||
|
|
@@ -717,7 +810,14 @@ def main(): | |||||||||||||||||||||||||||||
| script_dir = os.path.dirname(os.path.abspath(__file__)) | ||||||||||||||||||||||||||||||
| results_dir = os.path.join(script_dir, "results") | ||||||||||||||||||||||||||||||
| os.makedirs(results_dir, exist_ok=True) | ||||||||||||||||||||||||||||||
| output_path = os.path.join(results_dir, f"{args.dependency}-{args.version}-{timestamp}.csv") | ||||||||||||||||||||||||||||||
| if has_targets_list: | ||||||||||||||||||||||||||||||
| if os.path.exists(args.targets): | ||||||||||||||||||||||||||||||
| base_name = os.path.splitext(os.path.basename(args.targets))[0] | ||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||
| base_name = "targets" | ||||||||||||||||||||||||||||||
| output_path = os.path.join(results_dir, f"{base_name}-{timestamp}.csv") | ||||||||||||||||||||||||||||||
|
Comment on lines
+813
to
+818
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Calling
Suggested change
References
|
||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||
| output_path = os.path.join(results_dir, f"{args.dependency}-{args.version}-{timestamp}.csv") | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| write_csv_report(output_path, all_matches) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation has a few issues:
os.path.exists()on Windows with an inline JSON/YAML string (which contains invalid path characters like{,",:,[,]) will raise anOSErrorand crash the script.os.path.exists()returnsFalse, and the script falls back to parsing the path string as YAML. This succeeds (parsing as a plain string) but then fails theisinstance(raw_targets, dict)check, printing a confusing error about the input not resolving to a JSON object or YAML mapping instead of a "File not found" error."None"or"{'major': 3}"), leading to incorrect scanning behavior.We can resolve all of these by checking if the input looks like a file path first, handling non-existent files gracefully, and validating the parsed version types. Avoid broad except-blocks that silently pass; instead, log the exception to aid in debugging.
References
except Exception:blocks that silently returnNone. Instead, log the exception (e.g., usinglogger.warning) to aid in debugging and prevent masking underlying issues.