mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-09 00:04:10 +00:00
feat: recognize // volume: mounts in PHP scripts (#11018)
* feat: recognize `// volume:` mounts in PHP scripts Volume annotations were parsed for every language but PHP, so a PHP script could not mount a workspace volume. Two things stood in the way: PHP had no entry in the comment-prefix maps, and a PHP script opens with `<?php`, which ends the leading comment block the parsers scan before any annotation is read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3FR7iS9nRhpFt615cnuQ7 * fix: tolerate a PHP opener that carries code, drop the inert CLI hunk The open-tag skip matched `<?php` exactly, so `<?php declare(strict_types=1);` still ended the leading comment block and every annotation below it was silently ignored. Match the tag as a case-insensitive prefix and skip the whole line. The CLI local-graph hunk could never fire: PHP has no wasm asset parser, so `fallbackParse` handles it, and its own header scan stops at `<?php` — the script is dropped as a non-pipeline-member before any volume asset is read. Making only the CLI PHP-aware would also put the local graph out of parity with the deployed one, whose `parse_pipeline_annotations` stops there too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3FR7iS9nRhpFt615cnuQ7 * docs: correct the CLI mirror comment, state the own-line annotation rule Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3FR7iS9nRhpFt615cnuQ7 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
3e3a41d418
commit
f081fb1070
@@ -78,20 +78,26 @@ fn comment_prefix(lang: &ScriptLang) -> Option<&'static str> {
|
||||
| ScriptLang::Bun
|
||||
| ScriptLang::Bunnative
|
||||
| ScriptLang::Nativets
|
||||
| ScriptLang::Go => Some("//"),
|
||||
| ScriptLang::Go
|
||||
| ScriptLang::Php => Some("//"),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Mirror of the frontend `parseVolumeAnnotations` (infer.ts): `<prefix>
|
||||
/// volume: <path>` lines in the leading comment block, each an `rw` volume
|
||||
/// asset. Scanning stops at the first non-comment line (blank lines are
|
||||
/// skipped), exactly like the frontend.
|
||||
/// asset. Scanning stops at the first non-comment line; blank lines and PHP's
|
||||
/// opening tag line are skipped whole (the tag may carry code, so an annotation
|
||||
/// must sit on its own line below it), exactly like the frontend.
|
||||
fn parse_volume_annotations(content: &str, prefix: &str) -> Vec<AssetWithAltAccessType> {
|
||||
let mut volumes = Vec::new();
|
||||
for line in content.lines() {
|
||||
let trimmed = line.trim();
|
||||
if trimmed.is_empty() {
|
||||
if trimmed.is_empty()
|
||||
|| trimmed
|
||||
.get(..5)
|
||||
.is_some_and(|p| p.eq_ignore_ascii_case("<?php"))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
let Some(after) = trimmed.strip_prefix(prefix) else {
|
||||
@@ -258,4 +264,13 @@ mod tests {
|
||||
assert_eq!(vols[0].path, "my_vol");
|
||||
assert_eq!(vols[0].access_type, Some(AssetUsageAccessType::RW));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn php_volume_annotations_survive_the_open_tag() {
|
||||
let content = "<?php\n\n// volume: my_vol\nfunction main() {}\n";
|
||||
let got = effective_script_assets(&ScriptLang::Php, content, None).unwrap();
|
||||
let vols: Vec<_> = got.iter().filter(|a| a.kind == AssetKind::Volume).collect();
|
||||
assert_eq!(vols.len(), 1);
|
||||
assert_eq!(vols[0].path, "my_vol");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -179,11 +179,24 @@ pub fn interpolate_volume_name(
|
||||
result
|
||||
}
|
||||
|
||||
/// PHP's opening tag, which is case-insensitive and may be followed by code.
|
||||
fn is_php_open_tag(trimmed_line: &str) -> bool {
|
||||
trimmed_line
|
||||
.get(..5)
|
||||
.is_some_and(|p| p.eq_ignore_ascii_case("<?php"))
|
||||
}
|
||||
|
||||
/// Scans the leading comment block for `<prefix> volume: <name> <target>` lines,
|
||||
/// stopping at the first line that is neither blank nor a comment. A PHP script
|
||||
/// opens with `<?php`, which is not a comment, so that line is skipped like a blank
|
||||
/// one; otherwise the block would end before reaching any annotation. The whole line
|
||||
/// goes, since the tag may carry code (`<?php declare(strict_types=1);`) — so an
|
||||
/// annotation must sit on its own line, below the opener.
|
||||
pub fn parse_volume_annotations(content: &str, comment_prefix: &str) -> Vec<VolumeMount> {
|
||||
let mut volumes = Vec::new();
|
||||
for line in content.lines() {
|
||||
let trimmed = line.trim();
|
||||
if trimmed.is_empty() {
|
||||
if trimmed.is_empty() || is_php_open_tag(trimmed) {
|
||||
continue;
|
||||
}
|
||||
if !trimmed.starts_with(comment_prefix) {
|
||||
@@ -230,6 +243,17 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_php_volume_after_open_tag() {
|
||||
let content =
|
||||
"<?php declare(strict_types=1);\n\n// volume: mydata /tmp/data\nfunction main() {}";
|
||||
let result = parse_volume_annotations(content, "//");
|
||||
assert_eq!(
|
||||
result,
|
||||
vec![VolumeMount { name: "mydata".to_string(), target: "/tmp/data".to_string() }]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_multiple_volumes() {
|
||||
let content = "# volume: data1 /tmp/data1\n# volume: data2 /tmp/data2\n# volume: models /opt/models\ndef main():\n pass";
|
||||
|
||||
@@ -6412,7 +6412,8 @@ mount {{
|
||||
| ScriptLang::Bun
|
||||
| ScriptLang::Bunnative
|
||||
| ScriptLang::Nativets
|
||||
| ScriptLang::Go => "//",
|
||||
| ScriptLang::Go
|
||||
| ScriptLang::Php => "//",
|
||||
_ => "",
|
||||
};
|
||||
let raw_mounts = windmill_worker_volumes::parse_volume_annotations(&code, comment_prefix);
|
||||
|
||||
@@ -413,7 +413,13 @@ export function parseMuteAnnotations(content: string): {
|
||||
// Comment prefix for `volume:` annotations. Deliberately NOT `commentPrefix`
|
||||
// above (which returns `--` for SQL): volume annotations are only recognized for
|
||||
// the languages the backend/frontend recognize them for — mirrors
|
||||
// `asset_inference.rs:comment_prefix` and `infer.ts:getCommentPrefix` (SQL → none).
|
||||
// `asset_inference.rs:comment_prefix` and `infer.ts:getCommentPrefix` (SQL → none),
|
||||
// minus `php`. Those two recognize PHP, but a PHP script cannot reach this map: it
|
||||
// has no wasm asset parser, so `fallbackParse` handles it and that scan breaks on
|
||||
// the mandatory `<?php` opener, leaving `in_pipeline` false and the script dropped
|
||||
// as a non-member. Add `php` here together with PHP support in the pipeline
|
||||
// annotation scanners (backend `parse_pipeline_annotations` + its frontend mirror
|
||||
// + the header scans in this file), or the local graph desyncs from the deployed one.
|
||||
function volumeCommentPrefix(language: string): string | undefined {
|
||||
switch (language) {
|
||||
case "python3":
|
||||
|
||||
@@ -204,11 +204,16 @@ export type PreparedAssetsSqlQuery =
|
||||
| { columns: Record<string, string> } // e.g { id: "number", name: "text" }
|
||||
| { error: string; columns?: undefined } // error message if preparation failed
|
||||
|
||||
// Scans the leading comment block, stopping at the first line that is neither blank
|
||||
// nor a comment. A PHP script opens with `<?php`, which is not a comment, so that line
|
||||
// is skipped like a blank one; otherwise the block would end before reaching any
|
||||
// annotation. The whole line goes, since the tag may carry code (`<?php declare(…);`) —
|
||||
// so an annotation must sit on its own line, below the opener.
|
||||
function parseVolumeAnnotations(code: string, commentPrefix: string): AssetWithAccessType[] {
|
||||
const volumes: AssetWithAccessType[] = []
|
||||
for (const line of code.split('\n')) {
|
||||
const trimmed = line.trim()
|
||||
if (!trimmed) continue
|
||||
if (!trimmed || trimmed.slice(0, 5).toLowerCase() === '<?php') continue
|
||||
if (!trimmed.startsWith(commentPrefix)) break
|
||||
const after = trimmed.slice(commentPrefix.length).trim()
|
||||
const match = after.match(/^volume:\s*(\S+)/)
|
||||
@@ -233,6 +238,7 @@ function getCommentPrefix(language: SupportedLanguage | undefined): string | und
|
||||
case 'bunnative':
|
||||
case 'nativets':
|
||||
case 'go':
|
||||
case 'php':
|
||||
return '//'
|
||||
default:
|
||||
return undefined
|
||||
|
||||
Reference in New Issue
Block a user