Rust: Remove `argfile` dependency and read file list manually by paldepind · Pull Request #22916 · github/codeql

paldepind · 2026-10-28T13:55:42Z

The index-files.sh file in the extractors for Ruby, Ql, and Unified all pass the filename with the list of files to extract like this --file-list "$1".

However, in index-files.sh in the Rust extractor we pass it like @"$1". The @ is syntax to pass arguments by filename (I'm not sure what the origin of this notation is, but it's a somewhat well-known notation). We handle the @ by a dependency argfile.

The only thing we gain from introducing this dependency, using @ (that I was not familiar with), deviating from the other extractors, and special casing in the extractor_cli_config macro, is that we don't have to read the named file ourselves.

I don't think that's worth it, so this PR removes the argfile dependency and reads and parses the file manually (like the other extractors do). This gives closer alignment with the other extractors and less complexity. Furthermore, it turns out that, all things considered, this is in fact fewer lines of code than what we had with the dependency.

The dependency removal and generated Cargo/Bazel metadata are in the first commit. The code changes are in the second commit.

Copilot

🟡 Changes recommended

An unresolved minor finding notes that the configuration log omits the input-file count, making empty or partial extractions harder to diagnose.

1 open issue

Optional: the configuration log no longer reports how many input files were supplied, making empty… · New

What changed in this PR

This PR aligns the Rust extractor with other CodeQL extractors by replacing @argfile expansion with an explicit --file-list option.

Changes:

Update both launchers and read input paths directly from a newline-separated file.
Return QL test sources separately from configuration and remove input-specific macro handling.
Remove argfile and its unused transitive dependencies from Cargo and Bazel metadata.

File	Description
`rust/tools/index-files.sh`	Pass the file list through `--file-list`.
`rust/tools/index-files.cmd`	Use `--file-list` in the Windows launcher.
`rust/extractor/src/qltest.rs`	Return source paths instead of mutating configuration.
`rust/extractor/src/main.rs`	Read input paths from the configured file list.
`rust/extractor/src/config.rs`	Replace positional inputs with a file-list option.
`rust/extractor/macros/src/lib.rs`	Remove special handling for input paths.
`rust/extractor/Cargo.toml`	Remove the `argfile` dependency.
`MODULE.bazel`	Remove the `argfile` repository import.
`misc/bazel/3rdparty/tree_sitter_extractors_deps/crates.bzl`	Remove unused dependency and repository definitions.
`misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.os_str_bytes-7.2.0.bazel`	Delete unused transitive-dependency build metadata.
`misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.fs-err-3.3.0.bazel`	Delete unused transitive-dependency build metadata.
`misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.bazel`	Remove `argfile` aliases.
`misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.argfile-1.0.0.bazel`	Delete generated `argfile` build metadata.
`misc/bazel/3rdparty/tree_sitter_extractors_deps/argfile/BUILD.bazel`	Delete the unversioned dependency alias.
`misc/bazel/3rdparty/tree_sitter_extractors_deps/argfile-1.0.0/BUILD.bazel`	Delete the versioned dependency alias.
`Cargo.lock`	Remove `argfile` and its unused transitive dependencies.

_{Review effort: Max}

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35193a30-9bd2-4372-a2e5-65422a7779cf

andersfugmann

Code change looks good. Approvied.

Tbh, I'm torn. I understand the motivation to simplify the argument parsing (and reducing lines of code), but I actually think it's more elegant to accept the @<file> as an alternative way to provide arguments to a command line tool. If the argument is that we should unify the extractors, I'd prefer to adopt support for @<file> throughout.

I'm not strong on this, but recommend seeking input from #semmle_dev before merging.

andersfugmann · 2026-10-30T08:47:03Z

+    } else {
+        Vec::new()
+    };
    let start = Instant::now();


Not directly related to this PR, but should this not be at the very top of main?

Yes, that would make more sense. I've moved it up there.

andersfugmann · 2026-10-30T08:53:13Z

 use std::collections::HashSet;
+use std::fs::File;
 use std::hash::RandomState;
+use std::io::{self, BufRead, BufReader};


Is BufReadused?

Yes, but it's very hard to see. It's a trait and importing it puts its methods in scope and one of its methods is used.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68782cf6-b72d-4197-95b1-5c6003baf543 # Conflicts: # rust/extractor/src/main.rs

paldepind · 2026-10-30T13:51:56Z

Code change looks good. Approvied.

Tbh, I'm torn. I understand the motivation to simplify the argument parsing (and reducing lines of code), but I actually think it's more elegant to accept the @<file> as an alternative way to provide arguments to a command line tool. If the argument is that we should unify the extractors, I'd prefer to adopt support for @<file> throughout.

I'm not strong on this, but recommend seeking input from #semmle_dev before merging.

I think this would make sense if the extractor binary was often invoked "by hand" on the command line. But in practice I don't think we ever do that (I've never done it at least). Hence, I think it's more important to move towards alignment with the other Rust-based extractors and to keep the code simple.

I've merged main and moved the timer initialization, so the PR will have to be re-approved :)

github-actions Bot added the Rust Pull requests that update Rust code label Oct 28, 2026

paldepind marked this pull request as ready for review October 28, 2026 14:23

Copilot AI max review requested due to automatic review settings October 28, 2026 14:23

paldepind requested review from a team as code owners October 28, 2026 14:23

Copilot started reviewing on behalf of paldepind October 28, 2026 14:23

View session

Copilot AI reviewed Oct 28, 2026

View reviewed changes

Comment thread rust/extractor/src/main.rs Resolved

paldepind and others added 2 commits October 28, 2026 15:36

Rust: Remove argfile dependency

81c366a

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35193a30-9bd2-4372-a2e5-65422a7779cf

Rust: Read extraction inputs from an explicit file list

08eff93

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35193a30-9bd2-4372-a2e5-65422a7779cf

paldepind force-pushed the rust/remove-argfile branch from 38712e6 to 08eff93 Compare October 28, 2026 14:36

paldepind changed the title ~~Rust: replace argfile expansion with an explicit file list~~ Rust: Remove argfile dependency and read file list manually Oct 28, 2026

andersfugmann previously approved these changes Oct 30, 2026

View reviewed changes

paldepind added 2 commits October 30, 2026 14:44

Merge main into rust/remove-argfile

392fba6

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 68782cf6-b72d-4197-95b1-5c6003baf543 # Conflicts: # rust/extractor/src/main.rs

Rust: Create start timer at the top of main

be7c46b

paldepind dismissed stale reviews from andersfugmann via be7c46b October 30, 2026 13:47

Navigation Menu

Repository navigation

Rust: Remove `argfile` dependency and read file list manually - #22916

Rust: Remove `argfile` dependency and read file list manually#22916
paldepind wants to merge 4 commits into
mainfrom
rust/remove-argfile

paldepind commented Oct 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

andersfugmann left a comment

Uh oh!

andersfugmann Oct 30, 2026

Uh oh!

paldepind Oct 30, 2026

Uh oh!

andersfugmann Oct 30, 2026

Uh oh!

paldepind Oct 30, 2026

Uh oh!

paldepind commented Oct 30, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

📋 Server Request & Proxy Logs

⚙️ Pengaturan Proxy Target

Conversation

paldepind commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

andersfugmann left a comment

Choose a reason for hiding this comment

Uh oh!

andersfugmann Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

paldepind Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

andersfugmann Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

paldepind Oct 30, 2026

Choose a reason for hiding this comment

Uh oh!

paldepind commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paldepind commented Oct 28, 2026 •

edited

Loading

paldepind commented Oct 30, 2026 •

edited

Loading